-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Simplify get_flag() conditionals in get_abi_tag() [pep425tags] #6889
Simplify get_flag() conditionals in get_abi_tag() [pep425tags] #6889
Conversation
src/pip/_internal/pep425tags.py
Outdated
sys.version_info < (3, 8))) \ | ||
and sys.version_info < (3, 8): | ||
if sys.version_info < (3, 8) and get_flag( | ||
'WITH_PYMALLOC', lambda: impl == 'cp', warn=(impl == 'cp')): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would do is_cpython = impl == 'cp'
above and use it everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was planning on doing exactly this. :)
src/pip/_internal/pep425tags.py
Outdated
@@ -115,18 +115,12 @@ def get_abi_tag(): | |||
lambda: hasattr(sys, 'gettotalrefcount'), | |||
warn=(impl == 'cp')): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why we only warn for CPython, are these totally sure things on PyPy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For comparison, you may want to look at the newer packaging.tags to see if behavior like this got preserved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning is if they’re missing, so I’m guessing it’s because they aren’t known to be required on other implementations (so the warnings would risk being spurious).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In packaging.tags
the behavior did not get preserved, which may be a bug on Windows (filed at pypa/packaging#181).
That seems reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks! |
…itionals Simplify get_flag() conditionals in get_abi_tag() [pep425tags]
This simplifies a couple conditionals in
pep425tags.py
'sget_abi_tag()
function, as mentioned in this comment to PR #6874.