Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Migrate from hatch to uv #3723

Open
wants to merge 46 commits into
base: main
Choose a base branch
from
Open

Conversation

dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Dec 29, 2024

Will close #3577

Description

This PR transitions us from hatch to uv with a solution for the issue I raised in (#3577 (comment)).

Note

Don't be alarmed by the +3k additions, it is mostly in uv.lock
More info

Using uv

Install uv for the first time or run:

uv self update

Install python:

uv python install 3.12

Go to the root of the repo:

cd altair/

Initialize a new virtual env:

uv venv -p 3.12

Install/update dependencies:

uv sync --all-extras

Also see CONTRIBUTING

Task Runner

I've been following astral-sh/uv#5903 pretty closely and after experimenting rolling my own runner - I settled on https://github.com/taskipy/taskipy.

Usage

Works in a similar way to hatch run ..., with commands defined in [tool.taskipy.tasks].

Changes

Composition

Composing tasks uses "... && ...", rather than ["...", "..."]

Invoking

Invoking a task:

hatch run generate-schema-wrapper
uv run task generate-schema-wrapper

Alternatives

These were also mentioned in astral-sh/uv#5903.

I chose taskipy as:

  • has enough functionality for what we're doing
  • easy to understand
  • easiest to replace later

The last point is quite apparent when looking at the changes in (00f353a) vs (https://github.com/vega/altair/blob/ba836abb77b8e1eb8b5d194ed9442916ccdbf2a9/tasks.toml)

Tasks

https://ss64.com/bash/mkdir.html
We would want to see an error if `doc/` didn't exist. The docs wouldn't build as all the content is in that directory.

Working on migrating to a cross-platform shell that doesn't require this option anyway https://www.nushell.sh/commands/docs/mkdir.html
Decouples from `bash`, simplifies (#3577 (comment))
Provides 2 interfaces:
- `tasks.py`: to define using decorators
- `tasks.toml`: an example of defining in a similar way to `hatch` scripts

#3577 (comment)
Steps to reproduce:
- `uv self update` or install `uv` for the [first time](https://docs.astral.sh/uv/getting-started/installation/)
- `uv python install 3.12`
- `cd $ALTAIR_REPO_ROOT`
- `uv venv -p 3.12 --seed`
- `uv sync --all-extras`
Bug revealed after changing the `pytest` task to only contain `"pytest"`
- Add docs
- Handle semicolon joining in one place
- align `mkdir_cmd` with `mkdir -p` (https://ss64.com/bash/mkdir.html)
`*_cmd` functions and these variables are unrelated

Planning to namespace the functions, using `cmd`
All of these tools are now accessible under a single namespace, with discoverable docs
You can now safely run:
```py
import tools
tools.tasks.app.run(["lint", "format"])
```
Terms like "clean", "build", "publish" have another meaning when talking about the distribution
https://docs.astral.sh/uv/concepts/projects/build/
- Extra prep for string escaping while in a subprocess
- Use `--force` in `git add` to override (https://github.com/vega/altair/blob/7c2d97ffe80749a7cd1b24703547e40f4918172d/doc/.gitignore#L1)
- Raise a more informative error when a subprocess fails
All previous functionality is possible without this now.
Did some tidying up of what is left using (https://github.com/vega/vega-datasets/blob/369b462f7505e4ef3454668793e001e3620861ff/taplo.toml)

## Remaining sections
- `build`, `metadata`, `version` tables are used by `hatchling`
  - (https://hatch.pypa.io/latest/why/#build-backend)
- The `doc` environment is retained **only** due to how `_HatchRunner` is implemented
  - Otherwise we can remove that
  - (6e792a1)
- The test matrix functionality isn't something I've tried to reproduce, so left that in
  - Unsure how often others use it, but we could swap that out for `nox`
    - https://github.com/narwhals-dev/narwhals/blob/024d5d2294d1dcdd2e2f043d59eb03a3238c9e87/noxfile.py#L13
Comment on lines +144 to +151
if no_commit:
print(f"Unused commit message:\n {commit_message!r}")
elif branch != "main":
# FIXME: Unsure how to reproduce the RELEASING.md steps from this PR
# https://github.com/vega/altair/blob/main/RELEASING.md
print(f"Unable to push from {branch!r}.\n" "Must be on 'main'.")
else:
add_commit_push_github(commit_message)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXME

@mattijn I was wondering if you might be able to help me with this?
Looking at altair-viz.github.io(contributors), it seems like you've run these steps the most.

All the steps up this sequence are working:

# add commit, and push to github
git add . --all
git commit -m "$MSG"
git push origin master

These are the equivalent (with --force (404bb26)):

CMD_ADD = "git add . --all --force"
CMD_COMMIT = "git commit -m"
CMD_PUSH = "git push origin master"

Which are run in:

add_commit_push_github()

def add_commit_push_github(msg: str, /) -> None:
os.chdir(DOC_REPO_DIR)
print("Pushing ...")
# NOTE: Ensures the message uses cross-platform escaping
cmd_commit = *CMD_COMMIT.split(" "), msg
commands = (CMD_ADD, cmd_commit, CMD_PUSH)
for command in commands:
run_check(command)

Problem

Delete L146-L149 and it produces an error that I'm unsure how to solve.
AFAIK, it didn't seem to be related to permissions on https://github.com/altair-viz/altair-viz.github.io - but that is a blocker I'm expecting


RELEASING.md

altair/RELEASING.md

Lines 37 to 54 in 48e976e

7. Merge release branch into main, make sure that all required checks pass
8. On main, build source & wheel distributions. If you work on a fork, replace `origin` with `upstream`:
git switch main
git pull origin main
hatch clean # clean old builds & distributions
hatch build # create a source distribution and universal wheel
9. publish to PyPI (Requires correct PyPI owner permissions):
hatch publish
10. build and publish docs (Requires write-access to altair-viz/altair-viz.github.io):
hatch run doc:publish-clean-build
11. On main, tag the release. If you work on a fork, replace `origin` with `upstream`:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this can be resolved prior to merging to main.

100% sure it will need an assist from one of you fine people 🙂

@dangotbanned dangotbanned changed the title build(DRAFT): Migrate from hatch to uv build: Migrate from hatch to uv Jan 6, 2025
- Allows `uv run tools/tasks.py test` to be replaced with `uv run task test`
- Handles subprocess better for `pytest` and `sphinx-build`

Suggested in astral-sh/uv#5903 (comment)
dangotbanned added a commit that referenced this pull request Jan 8, 2025
Currently blocking `generate-schema-wrapper
e43e464
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Reverts a good chuck of new stuff added in this PR, prior to 00f353a

Next commit will refactor some of what is left - to simplify `[tool.taskipy.tasks]`
- Has parts of prior `sync_website.py`
- Utilizes for more concises commands in `[tool.taskipy.tasks]`
Easier to maintain and understand if this is in one place
- Add missing entries to `__all__`
- Add missing `copytree` doc
- Remove incorrect `dir_exists` doc
@dangotbanned dangotbanned marked this pull request as ready for review January 9, 2025 18:59
@dangotbanned
Copy link
Member Author

Important

I did a bit of a drive-by fix on README.md

The current instructions send you along this route to get to how to contribute:

I remember this confusing me when I first contributed and it happened again while I was updating the docs.
I hope that change makes sense to others.
I felt I needed to include it here to avoid having pip mess with uv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch from hatch to uv for package and environment management
1 participant