Repository navigation
RFC: restrict utils.xml's extension modules to CPython's Limited API - #18249
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
|
👋 Thank you for your draft pull request! Do you know that you can use |
3a6e4a6 to
34f4b8d
Compare
|
Oh I just realized that |
da78804 to
33fe44f
Compare
|
The compilation error on windows seems real. Will investigate. |
b8acdfd to
bd66828
Compare
bd66828 to
1ef1fd5
Compare
|
Hum, I'll take a step back and try to get building on Windows + Limited API stable first in #18252 |
1ef1fd5 to
8497527
Compare
utils.xml's extension modules to CPython's Limited APIutils.xml's extension modules to CPython's Limited API (⏰ wait for #18252 and #18253)
| return NULL; | ||
|
|
||
| if (PyType_Ready(&IterParserType) < 0) | ||
| IterParserType = PyType_FromModuleAndSpec((PyObject*)&moduledef, &IterParserType_spec, NULL); |
There was a problem hiding this comment.
I'm fairly sure the first argument should be the module (m) not moduledef
There was a problem hiding this comment.
of course ! I don't know what I was thinking (and I'm a bit worried that it seemed to work anyway ?). Thanks !
There was a problem hiding this comment.
I don't think it does much with the module except store the pointer somewhere. So it's unlikely to be an issue unless you try to use it.
8497527 to
cfec34a
Compare
utils.xml's extension modules to CPython's Limited API (⏰ wait for #18252 and #18253)utils.xml's extension modules to CPython's Limited API (⏰ wait for #18252)
d7127a5 to
0103a7f
Compare
2fb1ee3 to
f742109
Compare
utils.xml's extension modules to CPython's Limited API (⏰ wait for #18252)utils.xml's extension modules to CPython's Limited API
f742109 to
6746078
Compare
mhvk
left a comment
There was a problem hiding this comment.
I don't know all the details of things like Py_VISIT, but overall this looks good. Approving, though it would probably good if someone like @astrofrog could have a look too.
|
It might be good to add a comment about Py_VISIT but not critical so approving |
|
I added a call to Specifically, the source reference is https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_dealloc.
where I'll summarize this as an inline comment. |
6746078 to
83c8ece
Compare
|
Thanks! |
Description
Ref #18163