-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
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
Conversation
This uses a "def-like" structure: a PyModuleDef* that's not a valid Python object.
Co-Authored-By: Victor Stinner <vstinner@python.org>
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.
|
!buildbot AMD64.FreeBSD.Refleaks |
|
🤖 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:
|
Yes. There's a lot to add since the PEP calls for soft-deprecation of |
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
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.
| static inline PyModuleDef *_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; |
|
|
||
| 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.
|
Thank you for the review! |
|
Congrats :-) |
|
I created #141056 to fix the issue. |
|
Thank you! |
This adds the initial implementation of PEP-793.
See the issue for follow-up tasks.