gh-141004: Document missing PyCFunction* and PyCMethod* APIs
#141253
Conversation
bedevere-app
bot
added
awaiting core review
docs
There was a problem hiding this comment.
I suggested some updates so the tone is consistent (eg. We use trailing "s" chars like "returns" but we use "Get" instead of "Gets") so we should keep the same style for consistency and to improve reading flow
I also surrounded true and false with backticks
sharktide
left a comment
sharktide
left a comment
There was a problem hiding this comment.
In that case it looks fine to me / LGTM
Doc/c-api/structures.rst
Outdated
| available as :class:`types.BuiltinFunctionType` in the Python layer. | ||
| .. c:function:: int PyCFunction_Check(PyObject *f) |
There was a problem hiding this comment.
Keep a consistent name for the parameter. For _Check functions, I would prefer having op everywhere.
| .. c:function:: PyCFunction PyCFunction_GetFunction(PyObject *func) | ||
| Get the function pointer on *func* as it was passed to |
There was a problem hiding this comment.
Please indicate whether passing a non-function object sets an exception or causes a crash (to make the distinction with PyCFunction_GET_FUNCTION)
Doc/c-api/structures.rst
Outdated
| to the first argument of a :c:type:`PyCFunction`. In modules, this is the | ||
| module object. |
There was a problem hiding this comment.
"In modules, this is the module object" is unclear. I would either say "For modules"
There was a problem hiding this comment.
Let me know what you think of the new wording.
Doc/c-api/structures.rst
Outdated
| .. c:function:: int PyCFunction_Check(PyObject *f) | ||
| Return true if *f* is an instance of the :c:type:`PyCFunction_Type` type |
There was a problem hiding this comment.
Out of curiosity, is it the standard formulation?
There was a problem hiding this comment.
I'm surprised by the "instance of the X type". I would have just said "instance of X" but if we use the same sentence everywhere else, I'd prefer consistency.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
| If *func* is not a C function object, this fails with a | ||
| :class:`SystemError`. |
There was a problem hiding this comment.
This one is a regular exception right? so no need to mention it. What I wanted to know is whether passing func == NULL produced a SIGSEGV or a Python exception. Since passing NULL is "allowed" (but raises an exception) it's fine to remove the mention to SystemError.
There was a problem hiding this comment.
My advice is: check other Check functions to see how we formulate this. I don't want to be too pedantic but I want to be consistent in general.
3 participants
📚 Documentation preview 📚: https://cpython-previews--141253.org.readthedocs.build/en/141253/c-api/structures.html#implementing-functions-and-methods