Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Switch to setuptools_scm.#18971
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
dopplershift commentedNov 18, 2020
So the only downside to this approach is that for development installs, the version only updates when Matplotlib is different since it has compiled code and |
anntzer commentedNov 18, 2020
This can easily be fixed: in
We already hook the build_ext command to customize it, it's "probably" not too hard to customize it further to ensure it updates |
timhoffm commentedNov 18, 2020
Would it be reasonable to update the version in a dev-install at runtime if setuptools_scm is available? - I assume performance-wise that's the same as versioneer currently. |
anntzer commentedNov 18, 2020
Yes (that's basically the alternative I propose above). |
timhoffm commentedNov 18, 2020
Then I'd go with that. Feels more correct. We can always introduce an environment variable to deactivate the lookup should the need arise to do so. |
anntzer commentedNov 18, 2020
One point I forgot to ask is: do we want to add a runtime dependency on setuptools_scm (for what is a fairly rare use case)? or we can also just silently ignore the case of editable installs with setuptools_scm not present (unfortunately, I don't think "runtime dependency only for editable installs" is a thing). |
dopplershift commentedNov 18, 2020
|
anntzer commentedNov 21, 2020
No, setup_requires arenot available at runtime (they're only temporarily made present in the build environment via python -mvenv /tmp/testenvsource /tmp/testenv/bin/activatemkdir /tmp/foo;cd /tmp/fooecho'from setuptools import setup; setup(name="foo", use_scm_version=True, setup_requires=["setuptools_scm>=4"])'> setup.pygit init.&& git add.&& git commit -m"Initial commit."&& git tag v0.0pip install -ve.pip list
|
anntzer commentedJan 21, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Discussed on call today. Decision:
|
b1d250d to5d9a362Compare| `versioneer<https://github.com/warner/python-versioneer>`__ | ||
| which uses a format string in | ||
| .. [#]The tarball that is provided by GitHub is produced using `git archive`_. | ||
| We usesetuptools_scm_ which uses a format string in |
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.
| We usesetuptools_scm_ which uses a format string in | |
| We use`setuptools_scm`_ which uses a format string in |
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.
A quick test shows this isn't needed in rst?
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.
☑️ I've learned something new.
While the SphinxreStructuredText Primer only mentions the variant with backticks, thespecification indeed allows to leave them out. (Could somebody donate a tiny bit of CSS to the docutils folks? - The no-CSS style of the specs is quite unreadable, which is why I try to avoid going there.)
594ad02 to5ba072bComparetacaswell commentedMar 2, 2021
@anntzer I took the liberty of rebasing. |
timhoffm commentedMar 13, 2021
@tacaswell Is there a reason that you did not merge? |
tacaswell commentedMar 25, 2021
@timhoffm I think I approved, did the rebase, and then did not come back to it when CI passed. I'll rebase again and keep an eye on it this time! |
A few noteworthy points:- The contents in the sdist now exactly match what `git archive` would include; this avoids having to additionally keep track of things in MANIFEST.in (as noted in contributing.rst).- The `__version__` of an editable install is no longer recomputed at each import (as was done with `versioneer`, but only whenever the project is (re)installed; this can actually save quite a bit of time when working with many editable installs as each version recomputation requires shelling out a subprocess. (If really wanted we could keep the old behavior by optionally adding a runtime dependency on setuptools_scm and calling `setuptools_scm.get_version` in `matplotlib/__init__.py`.)
pllim commentedMay 10, 2021
Hello. I see that this is merged but |
Per yesterday's dev call.
A few noteworthy points:
The contents in the sdist now exactly match what
git archivewouldinclude; this avoids having to additionally keep track of things in
MANIFEST.in (as noted in contributing.rst).
The
__version__of an editable install is no longer recomputed ateach import (as was done with
versioneer, but only whenever theproject is (re)installed; this can actually save quite a bit of time
when working with many editable installs as each version recomputation
requires shelling out a subprocess. (If really wanted we could
keep the old behavior by optionally adding a runtime dependency
on setuptools_scm and calling
setuptools_scm.get_versioninmatplotlib/__init__.py.)Also note that I don't personally use the tarballs auto-created by github
or setuptools_scm_git_archive, so I'm not sure I got the config correct
on that side, I just copied what the docs say...
attn@tacaswell
Feel free to push to this PR; I'm mostly just posting an old patch I had...
PR Summary
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).