Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to setuptools_scm for automatic version numbers from git tags #856

Merged
merged 4 commits into from
Sep 6, 2019

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Jul 14, 2019

As mentioned in #678, we currently use versioneer to automatically figure out what the latest git tag is and provide a version number for satpy based on that. However, versioneer is unmaintained and it was suggested in pydata/xarray#2853 to switch to setuptools_scm. This PR attempts to do that.

References for things used and done in this PR:

https://pypi.org/project/setuptools-scm/
https://pypi.org/project/setuptools-scm-git-archive/
pypa/setuptools-scm#190 (comment)

A couple things to note:

  1. The new "unreleased" version numbers are a little different (ex. 0.16.2.dev1+gc4291966) but it depends if you current directory is dirty or not. Not a huge deal, but important to note.
  2. By default, as stated in the README for the project (see PyPI above), the tool uses pkg_resources to access the installation information and get the version number for the package. It has an option to write to a file instead which should improve startup/import of the satpy package. I use this in this PR and specifically ignore adding the file (satpy/version.py) to the repository so it is autogenerated every time.
  3. Also as stated on PyPI/README, the package can't generate the version number from github's zip releases unless you add setuptools-scm-git-archive too. So I added that to the installation steps too and followed its instructions (see second link above).
  4. The last link above points out an "issue" where setuptools-scm adds all repository files to the sdist tarball by default. I disagree with this behavior as it can easily mean extra unwanted files being included in the sdist tarball (even though the referenced issue mentions that they aren't installed on the users system, just put in the tarball only). I've included the referenced hack to not includes all files.

@djhoese djhoese added the enhancement code enhancements, features, improvements label Jul 14, 2019
@djhoese djhoese requested review from mraspaud and pnuu July 14, 2019 23:49
@djhoese djhoese self-assigned this Jul 14, 2019
@codecov
Copy link

codecov bot commented Jul 15, 2019

Codecov Report

Merging #856 into master will increase coverage by 0.54%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #856      +/-   ##
==========================================
+ Coverage   84.09%   84.64%   +0.54%     
==========================================
  Files         167      171       +4     
  Lines       24541    25048     +507     
==========================================
+ Hits        20637    21201     +564     
+ Misses       3904     3847      -57
Impacted Files Coverage Δ
satpy/__init__.py 85.71% <60%> (-9.03%) ⬇️
satpy/readers/viirs_edr_active_fires.py 89.28% <0%> (-1.46%) ⬇️
satpy/readers/generic_image.py 93.1% <0%> (-1.42%) ⬇️
satpy/writers/cf_writer.py 91.43% <0%> (-0.2%) ⬇️
satpy/scene.py 90.47% <0%> (-0.18%) ⬇️
satpy/tests/reader_tests/test_generic_image.py 97.67% <0%> (-0.12%) ⬇️
satpy/tests/writer_tests/test_mitiff.py 97.9% <0%> (-0.07%) ⬇️
satpy/readers/mersi2_l1b.py 96.38% <0%> (ø) ⬆️
satpy/composites/abi.py 100% <0%> (ø) ⬆️
satpy/readers/hdf4_utils.py 92.72% <0%> (ø) ⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cc29bb...b3e1a22. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 84.08% when pulling c429196 on djhoese:feature-setuptools-scm into 3cc29bb on pytroll:master.

@coveralls
Copy link

coveralls commented Jul 15, 2019

Coverage Status

Coverage increased (+0.6%) to 84.641% when pulling b3e1a22 on djhoese:feature-setuptools-scm into 3cc29bb on pytroll:master.

@@ -66,3 +66,7 @@ htmlcov
# vi / vim swp files
*.swp
.DS_STORE

# setuptools_scm files
# this should be generated automatically when installed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this include python setup.py develop ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything that runs setup.py should generate the necessary version files, however python setup.py X is technically deprecated. The preferred way of installing things is pip install . (full install) or pip install -e . (development mode).

However-however, pip install -e . won't work if we add a pyproject.toml without adding a --no-use-pep517. This is because I guess development mode is also deprecated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but does this version.py get generated with pip install -e . ?

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand correctly, so I have a question: it looks like the version.py file is now created on installation. But what happens when devs are working on the software, is the version.py updated for every commit ?

@djhoese
Copy link
Member Author

djhoese commented Aug 20, 2019

What is supposed to happen is that version.py gets updated every time you run an installation command. This means that if you did development mode your version number in python would stick with whatever it was when you installed it (I think). This is similar to other egg-info and other package metadata that won't update unless you reinstall the package.

The alternative is to have setuptools-scm check the git history every time the package is imported which can have a performance penalty. I also can't remember off hand how that works when outside of a git repository (wheel, sdist, github release tarball).

@mraspaud
Copy link
Member

mraspaud commented Aug 20, 2019

Looks like versioneer was recomputing the version every time satpy was imported: https://github.com/pytroll/satpy/blob/master/satpy/__init__.py#L54
I get that maybe that isn't optimal, so an alternative would be to trigger a version generation after every commit or pull or checkout. To that end, we could provide git hooks for satpy that update the version number. Maybe I prefer recomputing everytime after all

@djhoese
Copy link
Member Author

djhoese commented Aug 20, 2019

I can switch it to use pkg_resources as suggested on their documentation so it gets the version number at runtime. I'd prefer not doing it at runtime but it really isn't that big of a deal and I don't feel strongly about it. I wanted to avoid additional import time.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mraspaud mraspaud merged commit ffc21c2 into pytroll:master Sep 6, 2019
@djhoese djhoese deleted the feature-setuptools-scm branch September 6, 2019 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider using setuptools-scm instead of versioneer
3 participants