-
Notifications
You must be signed in to change notification settings - Fork 249
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
tags: add interpreter_tag #613
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Filipe Laíns <[email protected]>
def interpreter_tag(*, warn: bool = False) -> Optional[str]: | ||
interp_name = interpreter_name() | ||
if interp_name == "pp": | ||
return "pp3" |
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 haven't seen pp3
used anywhere by pypy. Do you know where it is used?
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.
pp3
is the generic PyPy 3 tag, it isn't used in many places. I can't remember any example where it's used from the top of my mind.
But actually, we probably want the tag with minor version here (eg. pp310
). I just noticed that given the code doesn't handle it right now, it doesn't think pp310-none-any
is a compatible tag on 3.10 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.
That comes specifically from PyPy and @mattip , so I would not want to change that logic without his approval.
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.
pp3-none-any
is a valid complete tag for PyPy, see #466. This is used in projects like vmprof that have a PyPy-specific pure python implementation for PyPy but need a c-extension implementation for CPython [1].
It is unclear what the correct value should be for an "interpreter_tag" function. Is the intention
- a "minimal" PEP 425 python tag for a pure-python package compatible with any version of python3? Then for CPython this should return
py3
, and for pypy it could return eitherpy3
orpp3
(if the pure-python code should only be used on PyPy) - a "versioned" PEP 425 python tag for a pure-python package that cannot be used on lower versions of python? Then for both CPython 3.9 and PyPy 3.9 this should return
py39
- a "versioned" PEP 425 python tag for a non-pure-python package? Then for CPython 3.9 this should return
cp39
and for PyPypp39
.
Without a docstring or any context where this should be used, it is difficult to figure out the author's intention. The name interpreter_tag
also does not help. I think it should be closer to python_tag
since that is the language of PEP 425.
[1] Note that the project has not released such a tagged wheel, since the project has not had a release since pip/wheel with #466 was merged and released.
As there are still open questions on the issue and there's a merge conflict I'm going to remove myself as a reviewer until those two things can be addressed to help keep my review queue clean. |
Closes #612