← 返回首页
gh-98586: add whatsnew entry covering limited-API outgoing vector calls, related documentation fix by wjakob · Pull Request #99056 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

gh-98586: add whatsnew entry covering limited-API outgoing vector calls, related documentation fix#99056

Merged
erlend-aasland merged 7 commits into
python:mainfrom
wjakob:whatsnew
Nov 6, 2022
Merged

gh-98586: add whatsnew entry covering limited-API outgoing vector calls, related documentation fix#99056
erlend-aasland merged 7 commits into
python:mainfrom
wjakob:whatsnew

Conversation

wjakob commented Nov 3, 2022
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

PR #98587 addressing issue #98586 lacked a "what's new" entry.

While making those changes, I noticed an inconsistency in how PY_VECTORCALL_ARGUMENTS_OFFSET is declared in the underlying Sphinx markup when compared to other macro constants like Py_TPFLAGS_HAVE_VECTORCALL. This PR fixes that as well, which should connect a few currently broken cross-references

PR python#98587 addressing issue python#98586 lacked a "what's new" entry. While making those changes, I noticed an inconsistency in how ``PY_VECTORCALL_ARGUMENTS_OFFSET`` is declared in the underlying Sphinx markup when compared to other macro constants like ``Py_TPFLAGS_HAVE_VECTORCALL``. This PR fixes that as well, which should connect a few currently broken cross-references

Copy link
Copy Markdown
Member

FYI, you should mention in the PR title (which is also the default commit message) what you are adding a What's New entry for...

CAM-Gerlach left a comment

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 Hide comment

Seems like these should be changed to use equivalent roles from the appropriate domain, though I'm not sure why the Python domain has data/const but the C domain doesn't, despite C actually having "real" constants and Python only having a convention, nor what the reason was for using construct from the "wrong" domain originally...

@AA-Turner , any expert insight here?

Comment thread Doc/c-api/call.rst Show resolved Hide resolved
Comment thread Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
Comment thread Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
wjakob changed the title gh-98586: add whatsnew entry, related documentation fix gh-98586: add whatsnew entry covering limited-API outgoing vector calls, related documentation fix Nov 3, 2022
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>

wjakob commented Nov 3, 2022

Copy link
Copy Markdown
Contributor Author

@CAM-Gerlach -- I updated the PR title (sorry) and merged the change with the missing article.

I am hesitant to make the domain-related changes you requested. Part of the reason is that I am following the pattern that generally seems to be used for macro constants. For example look at how Py_TPFLAGS_MANAGED_DICT is defined and referenced in the Sphinx doc (or any macro constant, as far as I can tell). If you wanted to move things over to the :c: Sphinx domain, then it would be good to do this in a concerted set of changes covering the entire documentation.

CAM-Gerlach left a comment

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 Hide comment

Okay, that makes sense for now I guess. In that case, LGTM, aside from one comment

Comment thread Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>

wjakob commented Nov 3, 2022

Copy link
Copy Markdown
Contributor Author

aside from one comment

Incorporated, thanks.

Comment thread Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
Comment thread Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

erlend-aasland 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 Hide comment

LGTM, thanks!

Related: we should really update the docs on best practice for writing exension modules using the Limited C API (there is already an issue for this).

erlend-aasland merged commit 57a4052 into python:main Nov 6, 2022
wjakob deleted the whatsnew branch November 14, 2022 09:56
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Footer

© 2026 GitHub, Inc.