Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-140550: Initial implementation of PEP 793 – PyModExport#140556
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
encukou commented Oct 24, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
This uses a "def-like" structure: a PyModuleDef* that's not a valid Python object.
encukou left a comment
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.
Thank you for the initial review!
Yes, but there's a lot of tests. And I've also renamed things that changed semantics, so unrelated changes/backports will conflict or fail to build, rather than merge cleanly at the Git level but be subtly broken.
| import unittest | ||
| import types |
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 couldn't find this in PEP 8.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
encukou commented Oct 28, 2025
!buildbot AMD64.FreeBSD.Refleaks |
bedevere-bot commented Oct 28, 2025
🤖 New build scheduled with the buildbot fleet by @encukou for commit 01d52ba 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140556%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
Uh oh!
There was an error while loading. Please reload this page.
encukou commented Oct 29, 2025
Yes. There's a lot to add since the PEP calls for soft-deprecation of |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Uh oh!
There was an error while loading. Please reload this page.
vstinner left a comment
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. Just please move _PyModule_GetGCHooks() to pycore_moduleobject.h.
| staticinlinePyModuleDef*_PyModule_GetDefOrNull(PyObject*arg){ | ||
| PyModuleObject*mod=_PyModule_CAST(arg); | ||
| if (mod->md_token_is_def){ | ||
| return (PyModuleDef*)((PyModuleObject*)mod)->md_token; |
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.
| return (PyModuleDef*)((PyModuleObject*)mod)->md_token; | |
| return (PyModuleDef*)mod->md_token; |
Uh oh!
There was an error while loading. Please reload this page.
| PyMODINIT_FUNC | ||
| FUNC_NAME(MODULE_NAME)(void) | ||
| INITFUNC_NAME(MODULE_NAME)(void) |
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.
Is it really needed to define a PyInit function if a modexport function is defined with slots? Developers may reuse this code as an example.
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.
Thanks for the catch! I think I had some platform-specific issues with the current version of setuptools. I added this to the TODO list in the issue; I'll get to it in a later PR.
Uh oh!
There was an error while loading. Please reload this page.
encukou commented Nov 5, 2025
Thank you for the review! |
589a03a into python:mainUh oh!
There was an error while loading. Please reload this page.
vstinner commented Nov 5, 2025
Congrats :-) |
vstinner commented Nov 5, 2025
I created #141056 to fix the issue. |
encukou commented Nov 5, 2025
Thank you! |
…honGH-140556) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Kumar Aditya <kumaraditya@python.org>
This adds the initial implementation of PEP-793.
See the issue for follow-up tasks.