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

experiment: migrate to uv as package manager #12817

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

danieleades
Copy link
Contributor

@danieleades danieleades commented Aug 23, 2024

This is an experiment in using UV as a package manager. This project currently does not use a package manager.

This PR is not intended to be merged as is, but to serve as a way to discuss a potential migration. Even if such a migration is desirable, it doesn't necessarily mean it should happen now. UV is very promising, but still very new.

No migration should be considered unless the following criteria can be met:

  • no loss of functionality of features compared to existing solution
  • comprehensive documentation of new workflows and migration path from old workflows
  • minimal effort for existing contributors to migrate to new workflow
  • no increased effort to onboard new contributors
    • for new developers not already familiar with the process of setting up the Sphinx dev environment manually, uv is significantly easier to get started, eg. git clone https://github.com/sphinx-doc/sphinx.git && cd sphinx && uv run pytest
  • clear demonstration of benefits of migration in terms of
    • CI performance
    • developer convenience
    • simplicity of configuration of CI jobs, scripts, etc.

Known Limitations

  • UV and Tox have overlapping functionality, which is not ideal. We could continue to use both, but ideally we'd solve both use cases with one tool
    • Both tools manage virtual environments. Seems silly to have two different approaches to the same problem
    • Tox is very slow to install dependencies compared to UV. This can be mitigated using the tox-uv plugin
    • UV can run tests against target python versions just like Tox, but doesn't have any built-in way of running against a matrix of python versions with a single command for local testing.

uv.lock Show resolved Hide resolved
Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

Thank you for trying this, and I appreciate experimenting with managing packaging, but to set expectations I don't think it's likely we'll merge anything like this unless it is (a) entirely optional for contributors and (b) backwards compatible (unless the entire ecosystem switches overnight).

The example I'd use is Tox, which is set up, but optional and not needed to make contributions.

A

Comment on lines +28 to 34
run: curl -LsSf https://astral.sh/uv/${{ env.UV_VERSION }}/install.sh | sh
- name: Render the documentation
run: >
uv run
sphinx-build
-M html ./doc ./build/sphinx
-vv
Copy link
Member

Choose a reason for hiding this comment

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

This will use the released version of Sphinx rather than the development version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why do you say that?

@danieleades
Copy link
Contributor Author

Thank you for trying this, and I appreciate experimenting with managing packaging, but to set expectations I don't think it's likely we'll merge anything like this unless it is (a) entirely optional for contributors and (b) backwards compatible (unless the entire ecosystem switches overnight).

The example I'd use is Tox, which is set up, but optional and not needed to make contributions.

A

What do you mean by "backwards-compatible" here?

@AA-Turner
Copy link
Member

What do you mean by "backwards-compatible" here?

Mainly identical functionality to the status quo.

A

@danieleades
Copy link
Contributor Author

Thank you for trying this, and I appreciate experimenting with managing packaging, but to set expectations I don't think it's likely we'll merge anything like this unless it is (a) entirely optional for contributors and (b) backwards compatible (unless the entire ecosystem switches overnight).

The example I'd use is Tox, which is set up, but optional and not needed to make contributions.

A

using UV would not be optional for contributors as it will be needed for bootstrapping the dev dependencies.

I think having "zero change to contributor workflow" should not be the goal. I think the goals should be:

  1. acceptable change to workflow for existing contributors
  2. easier onboarding for new contributors

I think goal 2. is absolutely satisfied. the workflow will change from:

git clone https://github.com/sphinx-doc/sphinx.git && cd sphinx
sudo apt update
sudo apt install python3-venv
python3 -m venv .venv && . ./.venv/bin/activate && python -m pip install ".[lint,test,docs]"
pytest

to:

git clone https://github.com/sphinx-doc/sphinx.git && cd sphinx
curl -LsSf https://astral.sh/uv/install.sh | sh
uv run pytest

I think that's much easier, and doesn't require new contributors to grok virtualenvs. That's without getting into the benefits of increased speed, dependency management (adding, updating, removing), reproducible environments, etc.

Goal 1. is far more subjective. The purpose of this PR is to provide an opportunity to play around and build consensus. If you have time i highly recommend cloning this and playing around with the UV cli

@picnixz
Copy link
Member

picnixz commented Aug 26, 2024

The fact that uv is not in distribution packages is what would stop me from helping. I personally prefer the virtual environment approach. I'm also unsure whether uv is known or not to contributors in general. This will be something new to teach (and to digest for old contributors). So I'm personally -0.5 (I wouldn't say outright -1 but I'm < 0).

In addition: "UV can run tests against target python versions just like Tox, but doesn't have any built-in way of running against a matrix of python versions with a single command for local testing." This is something that I like having for tox and is also very useful for autodoc related things (due to how typing changes across versions...). It's not really a blocker but I'd prefer not to switch to UV for now.

@chrisjsewell
Copy link
Member

chrisjsewell commented Aug 26, 2024

the workflow will change from

as already mentioned, you don't need all these steps for the existing configuration, and can still use uv with it, if you just use tox-uv (which I do)

@danieleades
Copy link
Contributor Author

the workflow will change from

as already mentioned, you don't need all these steps for the existing configuration, and can still use uv with it, if you just use tox-uv (which I do)

can you use tox without manually setting up the virtual environment and still have the IDE running within the dev environment?

@chrisjsewell
Copy link
Member

can you use tox without manually setting up the virtual environment and still have the IDE running within the dev environment?

yep

@danieleades
Copy link
Contributor Author

The fact that uv is not in distribution packages

what does this mean?

I personally prefer the virtual environment approach.

note that UV still uses a virtual environment, named '.venv' and stored in the project root. UV just manages the packages in it for you

@danieleades
Copy link
Contributor Author

danieleades commented Aug 26, 2024

can you use tox without manually setting up the virtual environment and still have the IDE running within the dev environment?

yep

can you use tox without manually setting up the virtual environment and still have the IDE running within the dev environment?

yep

might be something worth documenting in the 'internals' docs...

using tox-uv sounds like a good optimisation of the current framework. It doesn't address the package management features though- namely version resolution, reproducible environments (through dependency locking), etc.

@picnixz
Copy link
Member

picnixz commented Aug 26, 2024

what does this mean?

I need to install it via an sh script instead of an apt-get install. (should have said that there are no official repositories). When I said "would stop me from helping", it's not really me personally, but rather contributors that either want official pip packages or packages that are available from their distributions.

My main argument against uv (for now) is that you'll need to change the way you launch your tests. uv run pytest versus pytest for instance. I confess that I also never used uv so maybe it's a great tool! By the way, I'm not sure if the workflow:

sudo apt update
sudo apt install python3-venv
python3 -m venv .venv && . ./.venv/bin/activate && python -m pip install ".[lint,test,docs]"

is actually needed. I think venv is already bundled with python so you just need:

python3 -m venv .venv && source .venv/bin/activate && pip install -e .

@picnixz
Copy link
Member

picnixz commented Aug 26, 2024

I need to install it via an sh script instead of an apt-get install.

Nevermind it appears that there is a pip package as well.

@@ -134,6 +105,33 @@ exclude = [
"doc/_build",
]

[tool.uv]
dev-dependencies = [
Copy link
Member

Choose a reason for hiding this comment

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

this should wait on astral-sh/uv#5632, i.e. we shouldn't need to collapse all the dev dependencies into a single list

Copy link
Contributor Author

@danieleades danieleades Aug 26, 2024

Choose a reason for hiding this comment

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

could do. This might be premature optimisation though as the current dev dependencies-

  • install very quickly using UV
  • have no conflicts

based on that i think splitting them into separate groups might be a micro-optimisation

Copy link
Member

Choose a reason for hiding this comment

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

Nothing to do with speed: you are forcing all test environments to have unnecessary resolve constraints against unrelated packages, which can change the versions of packages you get

Copy link
Contributor Author

@danieleades danieleades Aug 26, 2024

Choose a reason for hiding this comment

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

yep, this is true in the general case. that also introduces the risk that different contributors resolve different versions depending on whether they are also building the docs, for example. Since even with dependency groups, the groups are not isolated from each other

Since even with dependency groups, the groups are not isolated from each other

i guess i'm speculating here, since the feature doesn't exist yet.

Still, i'm not sure it's a blocker. Certainly isn't causing any issues with the dependencies in sphinx today

Copy link

Choose a reason for hiding this comment

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

As of 0.4.27, uv supports separate groups. They still will constrain each other though, as the lockfile is for the entire project.

@danieleades
Copy link
Contributor Author

danieleades commented Aug 26, 2024

My main argument against uv (for now) is that you'll need to change the way you launch your tests. uv run pytest versus pytest for instance.

it doesn't have to, uv run is just a convenience.

UV still creates a virtual environment, so if you want to run pytest without the uv run prefix you absolutely can. These are equivalent (and both are supported when using UV):

source .venv/bin/activate
pytest

vs

uv run pytest

uv run is just shorthand for "run the following command inside the virtual environment"

@picnixz
Copy link
Member

picnixz commented Aug 26, 2024

Mmh. So to summarize, we could drop pip + venv and have uv instead? It could convince me and it reminds me of npm a bit. The argument of uv is that it's fast compared to pip. If this is the case, it could be an advantage. If we can make uv compatible with tox (i.e., switching to uv without changing the workflow of people using tox), then it will be good.

I need to play with uv myself to convince me but the concerns I have are not that much. Also, CPython switched to uv for docs so I'll probably need to adopt it at some point. The only concern is about the lack of matrix tests but we can probably supply some makefile rule that would do the job.

@danieleades
Copy link
Contributor Author

Mmh. So to summarize, we could drop pip + venv and have uv instead?

yep. it also replaces pyenv if you happen to use that.

If we can make uv compatible with tox (i.e., switching to uv without changing the workflow of people using tox), then it will be good.

The only concern is about the lack of matrix tests but we can probably supply some makefile rule that would do the job.

i agree. I don't love the idea of having multiple ways to manage virtualenvs and i think a makefile rule wrapping a matrix is probably not a terrible solution

| sh
- name: Install dependencies
run: uv pip install .[docs]
run: curl -LsSf https://astral.sh/uv/${{ env.UV_VERSION }}/install.sh | sh
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible: could we use the astral-sh/setup-uv GitHub Action here, instead of installing from a hosted scriptfile?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah: I notice we're already using the scriptfile version in a few workflows. Ignore me; I'll suggest that we migrate them collectively.

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.

6 participants