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.

Sign up for GitHub

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

Jump to bottom

Always use versioneer command classes in setup.py #948

Merged
merged 2 commits into from
Jul 25, 2022
Merged

Always use versioneer command classes in setup.py #948

merged 2 commits into from
Jul 25, 2022

Conversation

wence-
Copy link
Contributor

@wence- wence- commented Jul 12, 2022 •
edited
Loading

The GIT_DESCRIBE_TAG and VERSION_SUFFIX environment variables are used
to control the name and version of the created conda/pypi package.
They should, however, not be used to control the version of the
installed package by overriding the versioneer cmdclass since that
leaves an unmodified _version.py file in the installed package
directory. A consequence is that the version reported by
dask_cuda.__version__ is "0+unknown".

We cannot always use the versioneer-provided cmdclass unmodified since
PEP440 specifically forbids PyPI from accepting packages that have local
version identifiers (as used by versioneer). To get around this, when setup.py
detects it is building a PyPI package (GIT_DESCRIBE_TAG is in the environment),
patch the version returned from versioneer with a PyPI-compatible one.

While we're here, bring the conda version string into line with the
rest of the rapids ecosystem.

Closes #336.

@wence-
Always use versioneer command classes in setup.py
bd6c958 
The GIT_DESCRIBE_TAG and VERSION_SUFFIX environment variables are used
to control the name and version of the created conda/pypi package.
They should, however, not be used to control the version of the
installed package by overriding the versioneer cmdclass since that
leaves an unmodified _version.py file in the installed package
directory. A consequence is that the version reported by
dask_cuda.__version__ is "0+unknown". To fix this, always use the
versioneer-provided cmdclass.

While we're here, bring the conda version string into line with the
rest of the rapids ecosystem.
@wence- wence- requested a review from a team as a code owner July 12, 2022 15:33
@github-actions github-actions bot added the conda conda issue label Jul 12, 2022
@wence-
Copy link
Contributor Author

wence- commented Jul 12, 2022

Right now, dask-cuda installed from either pypi or the rapidsai and rapidsai-nightly conda channels reports:

$ python -c 'import dask_cuda; print(dask_cuda.__version__)'
0+unknown

I think this is the right fix but am not really able to test if this will break package version resolution. I also don't really know much about how the conda packages are built, so careful eyes are welcome.

@wence- wence- added bug Something isn't working 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels Jul 12, 2022
@wence-
Copy link
Contributor Author

wence- commented Jul 12, 2022

Related to #336, and this approach was previously used in #448 which had breakages, and was partially reverted in #466.

@wence-
Override versioneer.get_versions when GIT_DESCRIBE_TAG is defined
221c883 
Per PEP440, local version identifiers are disallowed for uploads to
public index servers. In CI, when we build pypi packages, we override
the versioneer-provided version with a PEP440-compatible version by
defining GIT_DESCRIBE_TAG and VERSION_SUFFIX. These are glued together
to produce a version which we then use to override the version
produced by versioneer.

This leaves us free to use the command classes provided by versioneer
in all circumstances. Closes rapidsai#336.
@wence-
Copy link
Contributor Author

wence- commented Jul 12, 2022

OK, I think I have a horrible hack that will work.

pentschev
pentschev approved these changes Jul 12, 2022
View reviewed changes
Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

Much nicer now:

$ python -c "import dask_cuda; print(dask_cuda.__version__)"
22.08.00a+30.g221c883.dirty

Thanks @wence- !

@pentschev
Copy link
Member

@gpucibot merge

@jakirkham
Copy link
Member

rerun tests

2 similar comments
@wence-
Copy link
Contributor Author

wence- commented Jul 21, 2022

rerun tests

@pentschev
Copy link
Member

rerun tests

@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.08@435dae8). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             branch-22.08    #948   +/-   ##
==============================================
  Coverage                ?   0.00%           
==============================================
  Files                   ?      16           
  Lines                   ?    2106           
  Branches                ?       0           
==============================================
  Hits                    ?       0           
  Misses                  ?    2106           
  Partials                ?       0

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 435dae8...221c883. Read the comment docs.

@rapids-bot rapids-bot bot merged commit 364cba2 into rapidsai:branch-22.08 Jul 25, 2022
@jakirkham
Copy link
Member

Let's keep an eye on the pre-releases on PyPI to make sure they are still getting published.

@wence- wence- deleted the wence/fix/conda-pypi-version branch July 26, 2022 09:57
@wence-
Copy link
Contributor Author

wence- commented Jul 26, 2022

Let's keep an eye on the pre-releases on PyPI to make sure they are still getting published.

Looks like it: https://pypi.org/project/dask-cuda/22.8.0a220726/

>>> import dask_cuda
>>> dask_cuda.__version__
'v22.08.00a220726'

@jakirkham
Copy link
Member

Thanks Lawrence! 🙏

@wence- wence- mentioned this pull request Sep 30, 2022
Merged
@wence- wence- mentioned this pull request Nov 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

@pentschev pentschev pentschev approved these changes

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team bug Something isn't working conda conda issue non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Odd __version__ reported
5 participants
@wence- @pentschev @jakirkham @codecov-commenter @ajschmidt8