Replace versioneer with setuptools-scm, migrate to pyproject.toml. #186

HexDecimal · 2023-05-22T23:21:17Z

#139 mentioned issues with Versioneer, so this PR completely removes it. pyproject.toml allows including versioning tools into the build system so having a bundled addon is no longer necessary.

Now _version.py is 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.py to build or develop. You use pip install -e . to install in development mode and python -m build to 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.

codecov · 2023-05-22T23:28:14Z

Codecov Report

Merging #186 (8d6325a) into master (d17fc69) will decrease coverage by 0.26%.
The diff coverage is 50.00%.

@@            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

Impacted Files	Coverage Δ
delocate/__init__.py	`66.66% <50.00%> (-33.34%)`	⬇️

📣 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.

matthew-brett

Thanks for doing this - it looks like a good cleanup.

matthew-brett · 2023-05-23T09:34:56Z

.github/workflows/pypi-deploy.yml

@@ -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.


Suggested change

# checkout@v1 is used to that tags are more relaible.

# checkout@v1 is used so that tags are more reliable. See:

# https:/actions/checkout/issues/290

Reference for this statement?

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.

OK - I added the URL to the suggestion.

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.

matthew-brett · 2023-05-23T09:37:13Z

delocate/__init__.py

 from .delocating import delocate_path, delocate_wheel, patch_wheel
 from .libsana import tree_libs, wheel_libs

-__version__ = _version.get_versions()["version"]
-del _version
+try:


Sorry for my ignorance here, but under what circumstances would this be triggered?

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 .

How about raising a warning in the block, possibly a deprecation warning?

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.

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.

matthew-brett

Consider deprecation and planned error after deprecation? Your call. PEP complaint, otherwise all good.

matthew-brett · 2023-05-23T10:50:32Z

delocate/__init__.py

 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(


But - what do you think about planning to raise an error after a period of deprecation?

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.

Your party, your rules! Just go for whatever you prefer.

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.

delocate/__init__.py

matthew-brett

Feel free to merge when you're happy ...

HexDecimal requested a review from matthew-brett May 22, 2023 23:21

Replace versioneer with setuptools-scm, migrate to pyproject.toml.

6211b25

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 May 23, 2023 09:20

matthew-brett reviewed May 23, 2023

View reviewed changes

Use latest checkout action.

9d744d8

matthew-brett reviewed May 23, 2023

View reviewed changes

Warn when delocate is run without version metadata.

8d6325a

HexDecimal force-pushed the setuptools-meta branch from 10c7d0e to 8d6325a Compare May 23, 2023 10:54

matthew-brett approved these changes May 26, 2023

View reviewed changes

HexDecimal merged commit 1c038e8 into matthew-brett:master May 26, 2023

HexDecimal deleted the setuptools-meta branch May 26, 2023 09:55

atalman mentioned this pull request May 30, 2023

Pin delocate to latest 0.10.4 release rathen then master pytorch/builder#1411

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace versioneer with setuptools-scm, migrate to pyproject.toml. #186

Replace versioneer with setuptools-scm, migrate to pyproject.toml. #186

HexDecimal commented May 22, 2023

codecov bot commented May 22, 2023 •

edited

Loading

matthew-brett left a comment

matthew-brett May 23, 2023 •

edited

Loading

HexDecimal May 23, 2023

matthew-brett May 23, 2023

HexDecimal May 23, 2023

matthew-brett May 23, 2023

HexDecimal May 23, 2023

matthew-brett May 23, 2023

HexDecimal May 23, 2023

matthew-brett May 23, 2023

matthew-brett left a comment

matthew-brett May 23, 2023

HexDecimal May 23, 2023

matthew-brett May 23, 2023

HexDecimal May 23, 2023

matthew-brett left a comment

	# checkout@v1 is used to that tags are more relaible.
	# checkout@v1 is used so that tags are more reliable. See:
	# https:/actions/checkout/issues/290

Replace versioneer with setuptools-scm, migrate to pyproject.toml. #186

Replace versioneer with setuptools-scm, migrate to pyproject.toml. #186

Conversation

HexDecimal commented May 22, 2023

codecov bot commented May 22, 2023 • edited Loading

Codecov Report

matthew-brett left a comment

Choose a reason for hiding this comment

matthew-brett May 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthew-brett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthew-brett left a comment

Choose a reason for hiding this comment

codecov bot commented May 22, 2023 •

edited

Loading

matthew-brett May 23, 2023 •

edited

Loading