-
Notifications
You must be signed in to change notification settings - Fork 266
Add compatibility for Pyodide-tagged wheels #804
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
base: main
Are you sure you want to change the base?
Conversation
We are changing our wheel platform tag to pyodide_2024_0 so we need to teach packaging to map Emscripten platform to this.
@mayeut @henryiii @pradyunsg would appreciate review on this from any of you (and running the workflow). |
Tests and coverage check now pass locally for me. |
I stumbled into this PR, somehow – I guess the tests are valid for only Python 3.12 and should be skipped for other Python versions, @hoodmane? |
Now, we also pre-emptively need |
+1 - it would be great if packaging installed from PyPi would just work out of the box in Pyodide |
I guess the only remaining parts here are to generalise the ABI tags, so that |
if pyodide_abi_version: | ||
yield f"pyodide_{pyodide_abi_version}_wasm32" | ||
yield from _generic_platforms() |
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.
Should this really yield the generic platform here as well, or is an else
missing, so that _generic_platforms
is yielded similar to the fallback in platform_tags
?
if pyodide_abi_version: | |
yield f"pyodide_{pyodide_abi_version}_wasm32" | |
yield from _generic_platforms() | |
if pyodide_abi_version: | |
yield f"pyodide_{pyodide_abi_version}_wasm32" | |
else: | |
yield from _generic_platforms() |
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 believe this is correct as is, since you want to also be able to match emscripten_...
tags. pyodide
is just a more specific tag.
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.
If I've understood @hoodmane's recent work correctly, all our forthcoming tags will be labelled with pyodide_YYYY_N
as per the Pyodide ABI information at https://pyodide.org/en/stable/development/abi.html#pyodide-2025-0-under-development and with PEP 783: https://peps.python.org/pep-0783/#specification that packaging
will implement through this PR. I think the distinction is important, as emscripten
is no longer the same as pyodide
since 2025_0
. In 2024, one could have both 2024_0
and emscripten_3_1_58
, which meant the same thing. 2025_0
does not have an emcripten_4_0_9
equivalent.
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 think we should keep it as is because emscripten_4_0_9
is the name of the platform and so it's the default tag. If anyone has wheels with this platform it makes sense to accept them.
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.
Yes, okay to keep it if you say so! :)
We are changing our wheel platform tag to pyodide_2024_0 so we need to teach packaging to map Emscripten platform to this.
cc @henryiii