FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-141004: Document missing `PyCFunction*` and `PyCMethod*` APIs by ZeroIntensity · Pull Request #141253 · python/cpython · GitHub

/ cpython Public

gh-141004: Document missing PyCFunction* and PyCMethod* APIs - #141253

Merged
ZeroIntensity merged 8 commits into
python:mainfrom
ZeroIntensity:document-pycfunction-pycmethod
Nov 16, 2025
ConversationCommits (8)ChecksFiles changed
Merged

gh-141004: Document missing PyCFunction* and PyCMethod* APIs#141253
ZeroIntensity merged 8 commits into
python:mainfrom
ZeroIntensity:document-pycfunction-pycmethod

Conversation

ZeroIntensity commented Nov 8, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

bedevere-app Bot added awaiting core review docs Documentation in the Doc dir skip news labels Nov 8, 2025
github-project-automation Bot moved this to Todo in Docs PRs Nov 8, 2025
bedevere-app Bot mentioned this pull request Nov 8, 2025
Open
sharktide reviewed Nov 15, 2025
View reviewed changes

sharktide left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread Doc/c-api/structures.rst Outdated
Comment thread Doc/c-api/structures.rst Outdated
Comment thread Doc/c-api/structures.rst Outdated
Comment thread Doc/c-api/structures.rst
Comment thread Doc/c-api/structures.rst
sharktide approved these changes Nov 15, 2025
View reviewed changes

sharktide left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

In that case it looks fine to me / LGTM

picnixz reviewed Nov 15, 2025
View reviewed changes
Comment thread Doc/c-api/structures.rst Outdated
available as :class:`types.BuiltinFunctionType` in the Python layer.


.. c:function:: int PyCFunction_Check(PyObject *f)

picnixz Nov 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Keep a consistent name for the parameter. For _Check functions, I would prefer having op everywhere.

Comment thread Doc/c-api/structures.rst
Comment thread Doc/c-api/structures.rst Outdated
Comment on lines +555 to +556
to the first argument of a :c:type:`PyCFunction`. In modules, this is the
module object.

picnixz Nov 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

"In modules, this is the module object" is unclear. I would either say "For modules"

ZeroIntensity Nov 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Let me know what you think of the new wording.

Comment thread Doc/c-api/structures.rst Outdated
Comment thread Doc/c-api/structures.rst Outdated
picnixz reviewed Nov 15, 2025
View reviewed changes
Comment thread Doc/c-api/structures.rst Outdated
Comment on lines +527 to +528
If *func* is not a C function object, this fails with a
:class:`SystemError`.

picnixz Nov 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

picnixz Nov 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

ZeroIntensity Nov 16, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

picnixz reviewed Nov 16, 2025
View reviewed changes
Comment thread Doc/c-api/structures.rst
picnixz approved these changes Nov 16, 2025
View reviewed changes
Comment thread Doc/c-api/structures.rst Outdated
Comment on lines +550 to +552



picnixz Nov 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We have 3 blank lines here. I'll let you edit this locally

ZeroIntensity enabled auto-merge (squash) November 16, 2025 19:20
ZeroIntensity added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Nov 16, 2025
ZeroIntensity merged commit be699d6 into python:main Nov 16, 2025
32 checks passed
ZeroIntensity deleted the document-pycfunction-pycmethod branch November 16, 2025 19:25
github-project-automation Bot moved this from Todo to Done in Docs PRs Nov 16, 2025

miss-islington-app Bot commented Nov 16, 2025

Copy link
Copy Markdown

Thanks @ZeroIntensity for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 16, 2025
pythongh-141004: Document missing PyCFunction* and PyCMethod* APIs (
93a24c9 
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>

bedevere-app Bot commented Nov 16, 2025

Copy link
Copy Markdown

GH-141637 is a backport of this pull request to the 3.14 branch.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Nov 16, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 16, 2025
pythongh-141004: Document missing PyCFunction* and PyCMethod* APIs (
4eff330 
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>

bedevere-app Bot commented Nov 16, 2025

Copy link
Copy Markdown

GH-141638 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Nov 16, 2025
ZeroIntensity added a commit that referenced this pull request Nov 16, 2025
[3.14] gh-141004: Document missing PyCFunction* and PyCMethod* AP…
c9eb5cb 
…Is (GH-141253) (GH-141637)

gh-141004: Document missing `PyCFunction*` and `PyCMethod*` APIs (GH-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>
ZeroIntensity added a commit that referenced this pull request Nov 16, 2025
[3.13] gh-141004: Document missing PyCFunction* and PyCMethod* AP…
e6f429c 
…Is (GH-141253) (GH-141638)

gh-141004: Document missing `PyCFunction*` and `PyCMethod*` APIs (GH-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>
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
pythongh-141004: Document missing PyCFunction* and PyCMethod* APIs (
80e2f49 
pythonGH-141253)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

picnixz picnixz approved these changes
+1 more reviewer
sharktide sharktide approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

Docs PRs
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL