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

Use setuptools_scm for versioning, fixes #1333 #1334

Merged
merged 5 commits into from
Nov 13, 2020
Merged

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented May 15, 2020

No description provided.

@codecov
Copy link

codecov bot commented May 15, 2020

Codecov Report

Merging #1334 (0efd086) into master (cb1dace) will increase coverage by 0.19%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1334      +/-   ##
==========================================
+ Coverage   90.78%   90.97%   +0.19%     
==========================================
  Files         188      189       +1     
  Lines       13562    13510      -52     
==========================================
- Hits        12312    12291      -21     
+ Misses       1250     1219      -31     
Impacted Files Coverage Δ
ctapipe/version.py 36.36% <33.33%> (-4.82%) ⬇️
ctapipe/_dev_version.py 60.00% <60.00%> (ø)
ctapipe/__init__.py 100.00% <100.00%> (ø)

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 cb1dace...0efd086. Read the comment docs.

@kosack
Copy link
Contributor

kosack commented May 15, 2020

please add a PR description of what this solves

@kosack
Copy link
Contributor

kosack commented May 26, 2020

could you merge in the master and fix conflicts? I'll take a look at this soon

kosack
kosack previously approved these changes Jun 16, 2020
kosack
kosack previously approved these changes Oct 23, 2020
@maxnoe
Copy link
Member Author

maxnoe commented Nov 9, 2020

@kosack I rechecked with the last astropy issues/PRs and implemented what they do, which makes sense I think.
So it behaves now like this:

  • If installed locally, we use setuptools_scm via _dev_version.py This file is excluded from distribution (sdist / wheel), so will not be included in a pypi upload. Not excluding setuptools scm for dists lead to some weird issues for astropy, so this is avoided now.

  • When building the dists, a file _version.py with just the version information is created. This is used for version information in releases

  • When neither is available, a clear warning is given and version = 0.0.0 is set. The only scenario this happens in should be if people try to install ctapipe through a release tarball autogenerated by github (this file cannot contain the autogenerated file and cannot contain git meta information, so this is just impossible). We should probably also add a warning at the bottom of each github release, that these tar balls should not be used to install ctapipe.

@maxnoe
Copy link
Member Author

maxnoe commented Nov 9, 2020

Ah, we could also think about uploading the sdist using the github releases plugin for pypi, so we have a functioning tarball on github (but just using pypi / conda is also fine I think)

@maxnoe maxnoe added this to the v0.10.0 milestone Nov 9, 2020
@maxnoe maxnoe merged commit 3689b52 into master Nov 13, 2020
@maxnoe maxnoe deleted the setuptools_scm branch November 13, 2020 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants