-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
bpo-46613: Add PyType_GetModuleByDef to the public & limited API #31081
Conversation
…ited API - Remove the underscore from the name (everywhere) - Add to limited API
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, LGTM.
whatsnew/3.11.rst shoud be updated? |
Yes, good catch. |
Here it is! |
Would it make sense to not add it to the limited API in Python 3.11, try to convert a few third party C extensions using it, and if everything goes well, add it to the limited API? |
It would! OTOH, addition to the limited API is also easy to revert, and the function itself already proved useful in stdlib. |
I think that makes sense. AFAICS, only a handful of devs (Christian Heimes, Hai Shi, Petr, Victor, me) have been using this function in the CPython code base. In this case, maybe it would make sense to try it out in the wild first. Who could we ask? |
For Py_NewRef/Py_XNewRef, I chose to add it to the stable ABI immediately. For PyType_GetModuleByDef(), I'm more scared because I introduced a bug and noticed it before your recent review. Sadly, only a minority of 3rd party C extensions are using heap types. Moreover, we discovered issues with heap types defined in the stdlib in Python 3.10:
These bugs made me even more careful than I was previously. Maybe PyType_GetModuleByDef() is fine, but it's just that heap types in general became "a minefield" if you don't pay attention. I love heap types, I want to use them everywhere. Maybe we need to enhance the documentation, add more warnings in PEP 630, and even implement new runtime checks (in development/debug mode?). For example, creating a type with Py_TPFLAGS_HAVE_GC but with no traverse function now raises an exception. |
Makes sense, but I'm skeptical of actually finding the issues (ones we can't fix in CPython patch release) unless this is in the limited API. Heap types -- and this function -- aren't very useful for existing projects if they aren't switching to the limited API.
Those are coming up! |
:c:func:`PyModule_GetState()` to get module state from slot methods (such as | ||
:c:member:`~PyTypeObject.tp_init` or :c:member:`~PyNumberMethods.nb_add`) | ||
and other places where a method's defining class cannot be passed using the | ||
:c:type:`PyCMethod` calling convention. |
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.
You may mention somewhere that this function only works on a type created by PyType_FromModuleAndSpec
.
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.
It works with any method that attaches the module to the class. Currently there's just one, but I've been adding new functions with extra arguments relatively fast. I'd rather not document one specific function.
Co-authored-by: Victor Stinner <vstinner@python.org>
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.
It's better with the correct return type in the doc :-)
I hope that we will be able to add it to the limited C API soon!
Thank you! |
…n#31081) * Make PyType_GetModuleByDef public (remove underscore) Co-authored-by: Victor Stinner <vstinner@python.org>
…n#31081) * Make PyType_GetModuleByDef public (remove underscore) Co-authored-by: Victor Stinner <vstinner@python.org> GitHub-Issue-Link: python/cpython#90771
…n#31081) * Make PyType_GetModuleByDef public (remove underscore) Co-authored-by: Victor Stinner <vstinner@python.org> GitHub-Issue-Link: python/cpython#90771
https://bugs.python.org/issue46613