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.
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.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Doc/c-api/structures.rst
Outdated
| 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.
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.
Ah, ok. NULL will result in a SIGSEGV, but a non-function will result in a SystemError. I updated the text to hopefully clarify that better.
Doc/c-api/structures.rst
Outdated
There was a problem hiding this comment.
We have 3 blank lines here. I'll let you edit this locally
ZeroIntensity
added
needs backport to 3.13
|
Thanks @ZeroIntensity for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
pythonGH-141253) (cherry picked from commit be699d6) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
GH-141637 is a backport of this pull request to the 3.14 branch. |
pythonGH-141253) (cherry picked from commit be699d6) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
GH-141638 is a backport of this pull request to the 3.13 branch. |
3 participants
📚 Documentation preview 📚: https://cpython-previews--141253.org.readthedocs.build/en/141253/c-api/structures.html#implementing-functions-and-methods