Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Drop setuptools-scm requirement in wheels#21820
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| # Installing from a git checkout. | ||
| ["setuptools_scm>=4"]ifPath(__file__).with_name(".git").exists() | ||
| else [] | ||
| # Installing from a git checkout that is not producing a wheel. |
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.
Why exactly is having this insetup_requires insufficient? Or is that only going to work withpyproject.toml?
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.
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.
See#18971 (comment) and#18971 (comment): the two options were either
(a) don't have a runtime dep on setuptools_scm, and accept that inplace installs get the wrong version if one doespip install -e . and then update the git repo (the version comes from_version.py and is only updated when setup.py is run, either directly (e.g. when recompiling) or via pip); or
(b) make setuptools_scm a dependency so that the version is always correct.
I was actually in favor of (a) (as described in that thread), but didn't feel that strongly about it. I guess that's another opportunity to go back to (a) :-)
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.
accept that inplace installs get the wrong version if one does pip install -e .
Do you mean that the wrong version gets reported, or that the install doesn't work? I think reporting the current commit in the version is nice, but hardly crucial.
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.
The wrong version gets reported if one doespip install -e and then updates the git repo (and even then, there are probably workarounds possible e.g. using git hooks...).
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.
As noted in#21783, my solution has been to import and use setuptools_scm, then fallback toimportlib_metadata. Given the fallback, it seemed fine to treatsetuptools_scm as an optional dependency and only list it as a "build" dependency. It's not perfect, but I've gone to statically listing all package metadata insetup.cfg.
tacaswell commentedDec 1, 2021
I agree with@QuLogic that this is the minimal fix for the wheels and adopting something more like@dopplershift 's metpy example for 3.6 |
QuLogic commentedDec 1, 2021
Here is a test build for wheels with this patch: |
tacaswell commentedDec 1, 2021
I also checked by hand locally with the right env set and did not have the dependency. |
dopplershift left a comment
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.
Fine enough for a bugfix release.
…820-on-v3.5.xBackport PR#21820 on branch v3.5.x (Drop setuptools-scm requirement in wheels)
PR Summary
Fixes#21783.
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).