Replace versioneer with setuptools-scm, migrate to pyproject.toml. #186
Conversation
Codecov Report
@@ Coverage Diff @@
## master #186 +/- ##
==========================================
- Coverage 95.00% 94.74% -0.26%
==========================================
Files 14 14
Lines 1100 1103 +3
==========================================
Hits 1045 1045
- Misses 55 58 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Versioneer has been completely removed. Now `_version.py` is generated during setup, but I've tweaked the script so that the package can run even without it. Setup metadata has been moved into pyproject.toml as well.
HexDecimal
force-pushed
the
setuptools-meta
branch
from
608f8f0 to
6211b25
Compare
.github/workflows/pypi-deploy.yml
Outdated
| @@ -9,17 +9,17 @@ jobs: | |||
| deploy: | |||
| runs-on: ubuntu-latest | |||
| steps: | |||
| # checkout@v1 is required since versioneer uses git describe. | |||
| # checkout@v1 is used to that tags are more relaible. | |||
There was a problem hiding this comment.
| # checkout@v1 is used to that tags are more relaible. | |
| # checkout@v1 is used so that tags are more reliable. See: | |
| # https://github.com/actions/checkout/issues/290 |
Reference for this statement?
There was a problem hiding this comment.
The checkout action has a history of messing up references to annotated tags which could affect scripts deriving versions from those tags but if the build system is just using any tag then it will probably work fine. actions/checkout#290
I think I can update the version here without issues, but you don't really know until you push a commit and it breaks.
There was a problem hiding this comment.
OK - I added the URL to the suggestion.
There was a problem hiding this comment.
I don't know. Seeing as the issue only affects annotated tags made me think this was a non-issue after all. If something happens then I'm familiar with some workarounds that might be better than keeping an older version of the action.
| from .delocating import delocate_path, delocate_wheel, patch_wheel | ||
| from .libsana import tree_libs, wheel_libs | ||
|
|
||
| __version__ = _version.get_versions()["version"] | ||
| del _version | ||
| try: |
There was a problem hiding this comment.
Sorry for my ignorance here, but under what circumstances would this be triggered?
There was a problem hiding this comment.
Not under normal circumstances, but if the module was imported directly from the repo without being installed in development mode then _version.py would be missing. Versioneer seemed to have a fallback for the case when a version was asked for with the package being setup and this was me accounting for that with the new setup.
It should also be possible to cover this with a test which installs the dependencies but not the package itself. I'd also be okay with removing this try/except clause and just telling everyone to be sure to use pip install -e .
There was a problem hiding this comment.
How about raising a warning in the block, possibly a deprecation warning?
There was a problem hiding this comment.
A RuntimeWarning might be appropriate, but also simply crashing when _version.py is missing is also an option. I'll go with whatever you want.
I think running the module without doing a development install is unrealistic and is going to crash on one of the imports of dependencies before it reaches this part.
I've had one of my own projects which required a trick like this but it was exceptional: It needed to import the package for doc generation but the package was too heavy to install on the doc generating servers.
There was a problem hiding this comment.
My preference would be to allow it for now, with a deprecation warning that we will break in the future - just because somebody may be using that mis-feature at the moment.
| from .delocating import delocate_path, delocate_wheel, patch_wheel | ||
| from .libsana import tree_libs, wheel_libs | ||
|
|
||
| try: | ||
| from ._version import __version__ | ||
| except ImportError: | ||
| __version__ = "0.0.0" | ||
| warnings.warn( |
There was a problem hiding this comment.
But - what do you think about planning to raise an error after a period of deprecation?
There was a problem hiding this comment.
This is a really niche usage of the package and the warning should make it clear what the correct usage is. This is just an elaborate alternative to crashing on ImportError because the package wasn't installed properly.
DeprecationWarning is too easy to miss, so it has to be one of the alternatives such as RuntimeWarning or FutureWarning.
Or the warning can be skipped and it just crashes when it isn't installed. This only affects developers of the package itself who should be installing it in development mode and will likely know what a missing _version module means.
Or silently fallback to __version__ = "0.0.0" since the version doesn't matter as much for development.
There was a problem hiding this comment.
Your party, your rules! Just go for whatever you prefer.
There was a problem hiding this comment.
Currently with as it is now this is the most explicit and most forgiving option: A clear warning saying "don't do this" while still allowing it.
I guess what's actually missing here is a CONTRIBUTING.md file saying to use pip install -e . to set this up for development.
HexDecimal
force-pushed
the
setuptools-meta
branch
from
10c7d0e to
8d6325a
Compare
#139 mentioned issues with Versioneer, so this PR completely removes it.
pyproject.tomlallows including versioning tools into the build system so having a bundled addon is no longer necessary.Now
_version.pyis generated during setup, but I've tweaked the script so that the package can still run without it.A lot of setup.py metadata has been moved into pyproject.toml as well. With a little more effort I think the entire thing can be moved to pyproject.toml but I stuck with the more important bits for now.
With this change you no longer use
setup.pyto build or develop. You usepip install -e .to install in development mode andpython -m buildto make distributions. I've automated this stuff some time ago and have now edited the release guide to reflect that.If this PR does too much then I can split it up.