Conversation
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6535 +/- ##
=======================================
Coverage 72.07% 72.07%
=======================================
Files 159 159
Lines 20633 20633
Branches 3273 3273
=======================================
Hits 14871 14871
Misses 5053 5053
Partials 709 709
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
PR switch project from Poetry to uv for dependency manage + build.
Changes:
- Move
pyproject.tomlfrom[tool.poetry]to PEP 621[project], switch build backend to hatchling, adddependency-groups. - Update docs to tell people use
uv syncinstead ofpoetry install. - Update GitHub Actions workflows to install
uvand runuv sync.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Replace Poetry config with PEP 621 project metadata, hatchling build config, uv dependency groups/extras. |
| extra/release.py | Read version from [project].version instead of Poetry table. |
| docs/faq.rst | Update “bleeding edge” install instructions to use uv sync. |
| docs/dev/plugins/index.rst | Remove Poetry mention/link in packaging tools note. |
| CONTRIBUTING.rst | Update contributor setup steps from Poetry to uv. |
| .github/workflows/ci.yaml | Replace Poetry install with uv sync in test matrix. |
| .github/workflows/lint.yaml | Replace Poetry install with uv sync, swap lockfile trigger to uv.lock. |
| .github/workflows/integration_test.yaml | Replace Poetry install with uv sync. |
| .github/workflows/make_release.yaml | Replace Poetry install with uv sync in release automation. |
Comments suppressed due to low confidence (6)
.github/workflows/integration_test.yaml:31
- grug see deps install is
uv sync, but workflow later runpoe testandpoe check-docs-links. need venv on PATH (useuv run poe ...or activate.venv). also linkcheck need docs extra; add--extra docs(or separate sync) so sphinx installed.
- name: Install dependencies
run: uv sync
- name: Test
env:
INTEGRATION_TEST: 1
run: poe test
- name: Check external links in docs
run: poe check-docs-links
.github/workflows/lint.yaml:72
- grug see
uv sync --only-group lintthen runpoe check-format. if uv install into.venv, ruff binary not on PATH for poe task. please runuv run poe check-format(or activate.venv) so task find ruff.
- name: Install dependencies
run: uv sync --only-group lint
- name: Check code formatting
# the job output will contain colored diffs with what needs adjusting
run: poe check-format
.github/workflows/lint.yaml:93
- grug see
uv sync --only-group lintthen runpoe lint .... same venv PATH problem: poe spawnruff, but shell not in.venv. run viauv run poe lint ...(or activate.venv).
- name: Install dependencies
run: uv sync --only-group lint
- name: Lint code
run: poe lint --output-format=github ${{ needs.changed-files.outputs.changed_python_files }}
.github/workflows/lint.yaml:115
- grug see
uv sync --only-group typingthen run mypy viapoe check-types .... if mypy installed into.venv, poe need run inside venv or mypy not found. please useuv run(or activate.venv) for this step.
- name: Install dependencies
run: uv sync --only-group typing
- name: Type check code
uses: liskin/gh-problem-matcher-wrap@v3
with:
linters: mypy
run: poe check-types --show-column-numbers --no-error-summary .
.github/workflows/lint.yaml:145
- grug see docs job do
uv sync --no-dev --extra docs, then runpoe format-docs(uses docstrfmt) andpoe lint-docs.--no-devlikely skip lint group where docstrfmt live, so step fail. also poe should run inside venv (uv run poe ...or activate.venv).
- name: Install dependencies
run: uv sync --no-dev --extra docs
- name: Add Sphinx problem matchers
run: |
echo "::add-matcher::.github/problem-matchers/sphinx-build.json"
echo "::add-matcher::.github/problem-matchers/sphinx-lint.json"
- name: Check docs formatting
run: poe format-docs --check
- name: Lint docs
run: poe lint-docs
.github/workflows/make_release.yaml:76
- grug see build job install deps then run
poe docs/poe changelog/poe build. same story: if uv create.venv, poe tasks not see sphinx/pandoc helpers unless run inside env. please switch these touv run poe ...(or activate.venv) in this step.
- name: Install dependencies
run: uv sync --no-group test --no-group lint --no-group typing --extra docs
- name: Install pandoc
run: sudo apt update && sudo apt install pandoc -y
- name: Obtain the changelog
id: generate_changelog
run: |
poe docs
{
echo 'changelog<<EOF'
poe --quiet changelog
echo EOF
} >> "$GITHUB_OUTPUT"
- name: Build a binary wheel and a source tarball
run: poe build
|
We might want to consider migrating to workspaces and separating beetsplug and beets. |
c474e11 to
8fb46c7
Compare
Hm, it may not apply to our use case?
|
|
Just attempted to build the package to see whether it suffers from the same issue (#5770): $ uv build
Building source distribution...
Building wheel from source distribution...
Successfully built dist/beets-2.9.0.tar.gz
Successfully built dist/beets-2.9.0-py3-none-any.whl
$ tar -vtf dist/beets-2.9.0.tar.gz | head -10
-rw-r--r-- 0/0 1631 2020-02-02 00:00 beets-2.9.0/beets/__init__.py
-rw-r--r-- 0/0 825 2020-02-02 00:00 beets-2.9.0/beets/__main__.py
-rw-r--r-- 0/0 4643 2020-02-02 00:00 beets-2.9.0/beets/config_default.yaml
-rw-r--r-- 0/0 646 2020-02-02 00:00 beets-2.9.0/beets/context.py
-rw-r--r-- 0/0 6766 2020-02-02 00:00 beets-2.9.0/beets/logging.py
-rw-r--r-- 0/0 1021 2020-02-02 00:00 beets-2.9.0/beets/mediafile.py
-rw-r--r-- 0/0 15166 2020-02-02 00:00 beets-2.9.0/beets/metadata_plugins.py
-rw-r--r-- 0/0 23681 2020-02-02 00:00 beets-2.9.0/beets/plugins.py
-rw-r--r-- 0/0 0 2020-02-02 00:00 beets-2.9.0/beets/py.typed
-rw-r--r-- 0/0 1540 2020-02-02 00:00 beets-2.9.0/beets/autotag/__init__.pyIt does not use the last file modification dates but AT LEAST it doesn't go back to 1970 like utterly useless poetry>2: $ uvx poetry'>2' build
Installed 45 packages in 143ms
Building beets (2.9.0)
Building sdist
- Building sdist
- Built beets-2.9.0.tar.gz
Building wheel
- Building wheel
- Built beets-2.9.0-py3-none-any.whl
$ tar -vtf dist/beets-2.9.0.tar.gz | head -10
-rw-r--r-- 0/0 1080 1970-01-01 01:00 beets-2.9.0/LICENSE
-rw-r--r-- 0/0 5316 1970-01-01 01:00 beets-2.9.0/README.rst
-rw-r--r-- 0/0 1631 1970-01-01 01:00 beets-2.9.0/beets/__init__.py
-rw-r--r-- 0/0 825 1970-01-01 01:00 beets-2.9.0/beets/__main__.py
-rw-r--r-- 0/0 1540 1970-01-01 01:00 beets-2.9.0/beets/autotag/__init__.py
-rw-r--r-- 0/0 18987 1970-01-01 01:00 beets-2.9.0/beets/autotag/distance.py
-rw-r--r-- 0/0 22492 1970-01-01 01:00 beets-2.9.0/beets/autotag/hooks.py
-rw-r--r-- 0/0 13618 1970-01-01 01:00 beets-2.9.0/beets/autotag/match.py
-rw-r--r-- 0/0 4643 1970-01-01 01:00 beets-2.9.0/beets/config_default.yaml
-rw-r--r-- 0/0 646 1970-01-01 01:00 beets-2.9.0/beets/context.pyThe issue was that |
How about using that instead? |
It hasn't been adopted as widely as uv. I've been resisting the migration to |
9809749 to
764ae7e
Compare
I think we could make it work. For example, we could split the core, database, and UI layers into separate packages. That would give us clearer boundaries and enforce a more explicit separation of responsibilities, though it might introduce some additional complexity in terms of packaging and coordination. |
|
I'd be interested to see how would it work and what benefits it may bring - but of course not as part of this PR but rather a separate piece of work that builds on this. |
Agreed! |
|
So what are we thinking regarding this? @semohr |
semohr
left a comment
There was a problem hiding this comment.
Thanks for taking on the work of migrating! Highly appreciated.
| @@ -28,12 +28,10 @@ jobs: | |||
| - uses: actions/checkout@v5 | |||
| - name: Install Python tools | |||
| uses: BrandonLWhite/[email protected] | |||
There was a problem hiding this comment.
Do we even need pipx anymore? I dont think it is used in the ci pipeline.
|
|
||
| .. code-block:: sh | ||
|
|
||
| $ python3 -m pip install --user pipx |
There was a problem hiding this comment.
Is the pipx recommendation still valid? I think poethepoet is now just a dev dependency. I dont think it is necessary anymore. Just running uv sync should do the job nicely.
There was a problem hiding this comment.
Introduction of uv does not change this. Both uv and poethepoet should be installed using pipx globally, outside of beets virtual environment.
There was a problem hiding this comment.
Why tho? uv is just a binary you drop somewhere. uv has no dependency on python, thus installing uv inside a pipx environment does not really do anything.
uv creates an environment for you, including poe if registered as dev dep.
I generally think pipx is not needed at all for the dev setup if we switch to uv.
There was a problem hiding this comment.
I use global uv and poethepoet binaries installed outside any specific project environment primarily because I maintain multiple projects.
Note we recommend using pipx to install these tools because they may not be available through the package manager of whatever distribution people use (for example, I doubt that poethepoet can be installed using apt).
There was a problem hiding this comment.
I get how you currently manage your dev envs now 👍 . Still, sorry to press on here.
Essentially we should not care how people install their uv (via pipx or just as a binary)`, right? We can give a recommendation ofc.
I think my misunderstanding boils down to that I don't really understand why we wont want to move poethepoet into the dev dependencies and have it be part of our dev environment (managed by uv).
What are the realistic reasons of having it externally managed? I don't really see any that's why I'm pressing here.
Moving poethepoet into our dev env will completely remove our pipx dependency in the ci and for the dev setup. It also lets us create a truly reproducible dev environment as we control the poe version. It should also eases the initial setup burden as people dont have to install another tool (just run uv sync and you are ready).
I for myself am still using conda for beets and would like to drop it in favor of just a singular uv venv if possible. One can install poe using uv tool install poethepoet too but than we wont have the guarantee that e.g. the CI poe version is the same version as my local poe version.
| ] | ||
|
|
||
| [tool.pipx-install] | ||
| poethepoet = ">=0.26" |
There was a problem hiding this comment.
This can go into the dev dependencies (see above). I dont think we should mix uv and pipx if not absolutely required.
There was a problem hiding this comment.
As I said above poethepoet should be installed globally by pipx outside beets venv - previously we had poetry here as well as it should also be installed outside.
Now, uv is installed by setup-uv action, so we don't need to provide it here any more.
There was a problem hiding this comment.
As I said above poethepoet should be installed globally by pipx outside beets venv
Why? Are there any dependency conflicts I'm missing?
My suggestion seems to be the recommended way to install poethepoet using uv
https://poethepoet.natn.io/installation.html#with-uv
There was a problem hiding this comment.
See the recommended way to install poethepoet: https://poethepoet.natn.io/installation.html#install-the-cli-globally-recommended
82d02d1 to
6067751
Compare
|
As the Fedora packager for beets, the switch from Poetry to standard PEP 621 metadata / hatchling looks generally fine from a downstream packaging perspective. The only thing I’d ask is that if workspace-style restructuring is considered later, please keep downstream/distribution builds in mind. A single normal sdist/wheel is straightforward for Fedora; multiple workspace members, editable local dependencies, or build/test assumptions that require uv sync can make isolated RPM builds much harder. |
67378d7 to
8b2d141
Compare
I think we've had enough of poetry so I let Claude Code migrate the code base to
uv. Let's see.Fixes: #5783