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

Update pip download to respect --python-version #6577

Merged
merged 2 commits into from
Jun 7, 2019

Conversation

cjerdonek
Copy link
Member

Fixes #5369.

@pypa-bot
Copy link

pypa-bot commented Jun 6, 2019

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will eligible for code review and hopefully merging!

@pypa-bot pypa-bot added the needs rebase or merge PR has conflicts with current master label Jun 6, 2019
@cjerdonek cjerdonek force-pushed the issue-5369-download-python-version branch from ab9c249 to 88990a3 Compare June 6, 2019 08:39
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jun 6, 2019
@cjerdonek cjerdonek force-pushed the issue-5369-download-python-version branch 3 times, most recently from 051b793 to 0444dda Compare June 6, 2019 08:51
@cjerdonek
Copy link
Member Author

@xavfernandez In this PR, similar to your suggestion here, I'm making e.g. 36 be interpreted as 3.6.0 for the purposes of the "Requires-Python" check. This is the purpose of the unit-tested complete_version_info () function. As you suggested, in a later PR we can start accepting dotted version strings for --python-version (and in particular major-minor-micro, not just major and major-minor).

Copy link
Member

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

In some places, we restrict py_version_info to a 3-int-tuple but apparently complete_version_info is capable of letting longer tuple pass unchanged, I think we should be consistent and might want to always have the same type and change the typing hints to Tuple(int, int, int) everywhere...

But after Requires-Python, it is likely that users will also want that pip uses --python-version when evaluating some environment markers like python_version, python_full_version & implementation_version.
And to do so, pip will need more than the 3-int tuple so we'll certainly end up with a 5-tuple as defined by https://docs.python.org/fr/3/library/sys.html#sys.version_info definition but this would certainly be for an other PR :)

@cjerdonek cjerdonek force-pushed the issue-5369-download-python-version branch 5 times, most recently from 2ceba72 to 494fad8 Compare June 6, 2019 18:32
@cjerdonek
Copy link
Member Author

cjerdonek commented Jun 6, 2019

@xavfernandez Thanks for the suggestion. I renamed complete_version_info() to normalize_version_info() to reflect that it can now also truncate, and I made it so that the functions used after normalize_version_info() are annotated with Tuple[int, int, int]. There was a bit of an annoyance in that I needed to use typing's cast() to get the fixed-length tuple to pass the mypy check, but it's working now.

By the way, here's an existing issue related to environment markers along the lines you said: #6117 It also has an associated PR (PR #6357) that I gave pointers on.

@cjerdonek cjerdonek force-pushed the issue-5369-download-python-version branch from 494fad8 to 8dbf88d Compare June 6, 2019 20:20
@cjerdonek
Copy link
Member Author

Thanks again for reviewing, @xavfernandez.

@cjerdonek cjerdonek merged commit 9c8b2ea into pypa:master Jun 7, 2019
@cjerdonek cjerdonek deleted the issue-5369-download-python-version branch June 7, 2019 20:55
@pradyunsg
Copy link
Member

Thanks for doing this @cjerdonek and reviewing this @xavfernandez! :)

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jul 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jul 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pip download --python-version fails when downloaded package doesn't support the Python version running pip
4 participants