-
-
Notifications
You must be signed in to change notification settings - Fork 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
Add guidelines for C API with output parameters #1128
Add guidelines for C API with output parameters #1128
Conversation
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, I just left some comments
PyFoo_Bar(PyObject **out) | ||
{ | ||
PyObject *value; | ||
int rc = foo_bar(&value); |
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.
Maybe you could mimic a legacy API call which returns NULL if not found and on error, and call PyErr_Occurred(). So the difference with the two APIs is even more obvious?
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.
See also python/cpython#108797. |
Do you need a different review? Or are you waiting for something? |
I think @encukou wants to wait until the sprint before any C API guidelines are updated in any kind of way. Correct me if I'm wrong, Petr. |
Oh, I wasn't aware of that. |
Some converters, like |
As I commented on python/cpython#108797, I think the guideline should care only about the general case, not the special case. There will be deviant APIs once in a while; that's ok. BTW, |
I concur with @erlend-aasland on that. |
FYI there is one public converter :-) I tried to move other private ones to the internal C API, but it's quite complicated: python/cpython#106320 (comment) I decided to give up in Python 3.13. I prefer to wait to see what's going on with the limited C API. |
I'll leave this to the C API workgroup. |
📚 Documentation preview 📚: https://cpython-devguide--1128.org.readthedocs.build/