Drop setuptools-scm requirement in wheels#21820
Conversation
| # Installing from a git checkout. | ||
| ["setuptools_scm>=4"] if Path(__file__).with_name(".git").exists() | ||
| else [] | ||
| # Installing from a git checkout that is not producing a wheel. |
There was a problem hiding this comment.
Why exactly is having this in setup_requires insufficient? Or is that only going to work with pyproject.toml?
There was a problem hiding this comment.
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 does pip 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.
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.
The wrong version gets reported if one does pip 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.
As noted in #21783, my solution has been to import and use setuptools_scm, then fallback to importlib_metadata. Given the fallback, it seemed fine to treat setuptools_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 in setup.cfg.
|
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 |
|
Here is a test build for wheels with this patch: |
|
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.
Fine enough for a bugfix release.
…820-on-v3.5.x Backport 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).