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

__version__ in pip wheel is mangled #361

Closed
abrammer opened this issue May 24, 2021 · 9 comments · Fixed by #363
Closed

__version__ in pip wheel is mangled #361

abrammer opened this issue May 24, 2021 · 9 comments · Fixed by #363
Assignees
Labels

Comments

@abrammer
Copy link
Contributor

abrammer commented May 24, 2021

Various installation and version checks

pip install pyresample==1.19.0 --no-binary :all: # don't use wheel, install from tar.gz
python -c "import pyresample; print(pyresample.__version__)"
1.19.0

pip install pyresample==1.19.0
python -c "import pyresample; print(pyresample.__version__)"
0+unknown

pip install pyresample==1.18.0
python -c "import pyresample; print(pyresample.__version__)"
0+unknown

pip install pyresample==1.17.0
python -c "import pyresample; print(pyresample.__version__)"
0+unknown

pip install pyresample==1.16.0
python -c "import pyresample; print(pyresample.__version__)"
0+unknown

pip install pyresample==1.15.0. # first version to include wheels
python -c "import pyresample; print(pyresample.__version__)"
0+unknown

pip install pyresample==1.14.0  # no wheels available install from tar.gz
python -c "import pyresample; print(pyresample.__version__)"
1.14.0

Problem description

The internal version attribute for pyresample from the wheels is mangled. Satpy utilizes this and will block functionality with the mangled version. https://github.com/pytroll/satpy/blob/378a2c563f647a7bb7ce74b417f288d0738d2762/satpy/resample.py#L182

Should satpy be querying the version in a different way or do the wheels need to be fixed?

Expected Output

the correct version number when calling pyresample.__version__

edit: added comments regarding the various install versions to document the method to my madness

@djhoese djhoese self-assigned this May 24, 2021
@djhoese djhoese added the bug label May 24, 2021
@djhoese
Copy link
Member

djhoese commented May 24, 2021

Thanks for filing this. This is...not good. We use versioneer for versioning in pyresample and a lot of us have been using conda-forge for a long time and conda-forge builds from source so that explains why we haven't seen it there.

So "good news" is I can reproduce it on my local machine. I'll try to figure this out in the next day or so.

@djhoese
Copy link
Member

djhoese commented May 24, 2021

Ah also just discovered that the reason 1.14.0 works is that we don't have any published wheels for that version. 1.15.0 was the first one we started doing it.

@abrammer
Copy link
Contributor Author

re 1.14 oh yeah, sorry. That's why I went back to 1.14 to show that it's been a problem since the wheels were introduced.

@djhoese
Copy link
Member

djhoese commented May 24, 2021

Generating a wheel locally seems to be bad as well if you are not in the git repository directory. So at least I can test it. I wonder if we just have versioneer configured poorly.

@abrammer
Copy link
Contributor Author

abrammer commented May 24, 2021

Updating to versioneer 0.19 seems to resolve the issue. I think it's related to this PR python-versioneer/python-versioneer#171 which although old was only incorporated into the semi recent 0.19 release.

@djhoese
Copy link
Member

djhoese commented May 25, 2021

Nice catch. That explains some of the confusion I had when reading the install instructions https://github.com/python-versioneer/python-versioneer/blob/master/INSTALL.md as it seems we don't have a versionfile_build set.

@mraspaud do you have any opinions on this? Should we try updating versioneer and review how we have it configured? Or we could switch to setuptools_scm to match the other pytroll packages I've switched to using it.

@mraspaud
Copy link
Member

I'm good with both solutions. Maybe upgrading versioneer is the most straightforward, so we can do that and only switch to setuptools_scm when we really need to?

@abrammer do you want to take a shot at a PR for this?

@abrammer
Copy link
Contributor Author

yeah, I can submit a PR. Also, noticed that just including the file on versionfile_build fixes the issue, without touching the rest of versioneer. I guess that's simplest if there are thoughts of removing versioneer in the future anyway.

Played around with adding tests to the deploy.yaml as well, so that the wheels are actually tested before upload, would need to also add a test on .__version__ to actually catch this particular issue though. Any thoughts on that, could open that up as a separate PR.

@djhoese
Copy link
Member

djhoese commented May 25, 2021

If you can create a PR for the versionfile_build change that would be good and then we can see how hard it would be to make a test for the wheel __version__ once that's created. I'd be OK with both changes being in the same PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants