-
Notifications
You must be signed in to change notification settings - Fork 38
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
Migrate from poetry & tox to uv #950
base: master
Are you sure you want to change the base?
Conversation
72c73d2
to
15c8b5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR, thanks! Just a few nits (mostly)
Hey @kevinkjt2000, if you're not too busy, could you review this? It's a pretty major move and affects all contributors, so I wanted your opinion as well. Especially since you haven't commented on the associated issue either. |
Can we move docs dependencies to a separate workspace? Sphinx does drop python versions fast |
b353c92
to
af4d322
Compare
Co-authored-by: Kevin Tindall <kevinkjt2000@users.noreply.github.com>
I don't think a separate workspace would be particularly beneficial here, we could add a version restriction in the dependency specification for sphinx, just like we do with
From what I understood about uv workspaces, the main goal of them is to create "inner dependencies", e.g. a package that's split into several submodules, that the main pkg depends on. Our docs don't necessarily fit this very well, as they're only needed during development. I'll admit that I haven't touched uv workspaces before though, so I might be missing something here. If you think they would be the right solution here though, I'd be fine with you pushing into this branch and adding that, or wait until this is merged and submit it as a follow-up PR. |
If it works - let's do it. I also don't know anything about uv workspaces, perhaps instead of workspaces we could create a separate uv project in our docs' directory? Will try after this gets merged |
This changes our primary dependency management tool from
poetry
touv
.Alongside, it also drops
tox
, asuv
doesn't have the equivalent of apoetry-tox
dependency, and I didn't really feel like bothering with getting it to work, considering there were plans to remove it anyways (#716). P.S. Sorry for taking this over Kevin, but the PR was stale for a long time now, so hope you don't mind, I did mark you as a co-author on one of the commits, as I pretty much just copied your work on the docs adjustments & poe tasks.I've already mentioned some of the reasons why
uv
is likely going to be a better choice for us in the associated issue: #925.Closes: #925, #488
Superseds & closes: #716
Some notes:
poetry shell
, so venv activation should be done in the standard way, as it would be with thevenv
module alone (source .venv/bin/activate
).v
). We can do this both from the GitHub UI or the CLI. After this is merged, it will also be necessary to change and maintain theversion
value in thepyproject.toml
. Even though I did find an equivalent topoetry-dynamic-versioning
, which was doing this for us automatically before:uv-dynamic-versioning
, looking at the GitHub stats of this project, it's not very popular, and I'd rather not introduce such a dependency. I did however add a check in the CI that ensures the version in pyproject.toml was in fact bumped before attempting a release (The release would fail anyways as PyPI would refuse it, but this way, it's at least cleaner what happened)pyproject.toml
file. This is handy when bumping the project version, as it's easy to forget about runninguv lock
afterwards, to update project version there too.What's a little bit unfortunate is how the(No longer the case, UV now has:uv sync
command handles groups, as there's no easy way to say "exclude all groups and only install these ones". Well, there is:--only-group <ONLY_GROUP>
, however, it only works with a single group and using it only installs the dependencies from this group, and I do mean only, it doesn't even install the project & it's dependenceis. From what I read in the docs &uv sync --help
, I didn't see any way to use this flag & install the project dependencies too. This means that we could either make all of the groups optional, and move the burden up to the contributors installing the project to include the--all-groups
or--group <GROUP>
for each needed group, or do what I did here, where all the groups are included by default and the CI workflows then exclude those that aren't needed. I will admit that this feels a bit awkward in comparison to poetry there and if we add more dependency groups, it would require updating the commands to exclude those (if desired). I'm fine with making the groups optional instead, if you think that'd be better.--no-default-groups
; see: 2f3e317)minecraft
,protocol
keywords. (This will show up on PyPI)docsutils
dependency to<0.21
, as later versions break them2r2
parser. See: docutils 0.21 has removed nodes.reprunicode() and therefore building documentation using m2r2 fails CrossNox/m2r2#68. I've just glanced over this, but it seems like m2r2 might be unmaintained, someone in that thread did mention an alternative: myst-parser. It might be worth it to look into this a bit more.