Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
ZeroIntensity merged 8 commits into from
Nov 16, 2025
Merged

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

ZeroIntensity merged 8 commits into from
Nov 16, 2025

Conversation

@ZeroIntensity
Copy link
Member

@ZeroIntensityZeroIntensity commented Nov 8, 2025•
edited
Loading

@bedevere-appbedevere-appbot added awaiting core review docs Documentation in the Doc dir skip news labels Nov 8, 2025
@bedevere-appbedevere-appbot mentioned this pull request Nov 8, 2025
Open
sharktide
sharktide reviewed Nov 15, 2025
View reviewed changes
Copy link
Contributor

@sharktidesharktide left a comment •
edited
Loading

Choose a reason for hiding this comment

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

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
sharktide approved these changes Nov 15, 2025
View reviewed changes
Copy link
Contributor

@sharktidesharktide left a comment

Choose a reason for hiding this comment

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

In that case it looks fine to me / LGTM

picnixz
picnixz reviewed Nov 15, 2025
View reviewed changes
Doc/c-api/structures.rst Outdated
available as :class:`types.BuiltinFunctionType` in the Python layer.
.. c:function::intPyCFunction_Check(PyObject *f)
Copy link
Member

@picnixzpicnixzNov 15, 2025

Choose a reason for hiding this comment

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

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

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.
Copy link
Member

@picnixzpicnixzNov 15, 2025

Choose a reason for hiding this comment

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

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

Copy link
MemberAuthor

@ZeroIntensityZeroIntensityNov 15, 2025

Choose a reason for hiding this comment

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

Let me know what you think of the new wording.

picnixz
picnixz reviewed Nov 15, 2025
View reviewed changes
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`.
Copy link
Member

@picnixzpicnixzNov 15, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Member

@picnixzpicnixzNov 15, 2025

Choose a reason for hiding this comment

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

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.

Copy link
MemberAuthor

@ZeroIntensityZeroIntensityNov 16, 2025

Choose a reason for hiding this comment

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

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
picnixz reviewed Nov 16, 2025
View reviewed changes
picnixz
picnixz approved these changes Nov 16, 2025
View reviewed changes
Doc/c-api/structures.rst Outdated
Comment on lines 550 to 552
Copy link
Member

@picnixzpicnixzNov 16, 2025•
edited
Loading

Choose a reason for hiding this comment

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

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

@ZeroIntensityZeroIntensity enabled auto-merge (squash) November 16, 2025 19:20
@ZeroIntensityZeroIntensity added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Nov 16, 2025
@ZeroIntensityZeroIntensity merged commit be699d6 into python:mainNov 16, 2025
32 checks passed
@ZeroIntensityZeroIntensity deleted the document-pycfunction-pycmethod branch November 16, 2025 19:25
@github-project-automationgithub-project-automationbot moved this from Todo to Done in Docs PRsNov 16, 2025
@miss-islington-app
Copy link

miss-islington-appbot commented Nov 16, 2025

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
@ZeroIntensity@picnixz@miss-islington
pythongh-141004: Document missingPyCFunction*andPyCMethod*APIs (
93a24c9
pythonGH-141253) (cherry picked from commit be699d6) Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app
Copy link

bedevere-appbot commented Nov 16, 2025

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

@bedevere-appbedevere-appbot 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
@ZeroIntensity@picnixz@miss-islington
pythongh-141004: Document missingPyCFunction*andPyCMethod*APIs (
4eff330
pythonGH-141253) (cherry picked from commit be699d6) Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app
Copy link

bedevere-appbot commented Nov 16, 2025

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

@bedevere-appbedevere-appbot 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
@miss-islington@ZeroIntensity@picnixz
[3.14]gh-141004: Document missingPyCFunction*andPyCMethod*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 <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
ZeroIntensity added a commit that referenced this pull request Nov 16, 2025
@miss-islington@ZeroIntensity@picnixz
[3.13]gh-141004: Document missingPyCFunction*andPyCMethod*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 <[email protected]> Co-authored-by: Bénédikt Tran <[email protected]>
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
@ZeroIntensity@picnixz@StanFromIreland
pythongh-141004: Document missingPyCFunction*andPyCMethod*APIs (
80e2f49
pythonGH-141253) Co-authored-by: Bénédikt Tran <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixzpicnixzpicnixz approved these changes

+1 more reviewer

@sharktidesharktidesharktide approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docsDocumentation in the Doc dirskip news

Projects

Docs PRs
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ZeroIntensity@picnixz@sharktide