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

tags: add interpreter_tag #613

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

FFY00
Copy link
Member

@FFY00 FFY00 commented Nov 9, 2022

Closes #612

Signed-off-by: Filipe Laíns <lains@riseup.net>
def interpreter_tag(*, warn: bool = False) -> Optional[str]:
interp_name = interpreter_name()
if interp_name == "pp":
return "pp3"
Copy link

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?

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Contributor

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 either py3 or pp3 (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 PyPy pp39.

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.

@brettcannon
Copy link
Member

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.

@brettcannon brettcannon removed their request for review June 9, 2023 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add packaging.tags.interpreter_tag
5 participants