Skip to content
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

bpo-43795: PEP 652 user documentation #25668

Merged
merged 14 commits into from
May 11, 2021
Merged

bpo-43795: PEP 652 user documentation #25668

merged 14 commits into from
May 11, 2021

Conversation

encukou
Copy link
Member

@encukou encukou commented Apr 27, 2021 •
edited by miss-islington
Loading

  • Reformat the C API and ABI Versioning page (and extend/clarify a bit)
  • Rewrite the stable ABI docs into a general text on C API Compatibility
  • Add a list of Limited API contents, and notes for the individual items.
  • Replace Include/README.rst with a link to a devguide page with the same info

https://bugs.python.org/issue43795

Automerge-Triggered-By: GH:encukou

encukou added 2 commits April 27, 2021 21:16
@encukou
Doc: Reformat the C API and ABI Versioning page
28e34ab 
Mark up all the version macros, adding them to the index and allowing
links to them.

Clarify that these are resolved at build time.

Add an example for Python 3.10. Some people might assume this is 0x10,
not 0xA, based on how well PY_VERSION_HEX "reads" in hex.

Link to the (upcoming) page on API and ABI stability.
@encukou
Add Stable ABI (PEP 652) documentation
61cf858
@encukou encukou added docs Documentation in the Doc dir skip news DO-NOT-MERGE labels Apr 27, 2021
@encukou encukou mentioned this pull request Apr 29, 2021
Merged
encukou and others added 5 commits April 29, 2021 17:06
@encukou
Mark up Py_LIMITED_API as a macro
b9dc6d2
@encukou
Replace Include/README.rst with a link to the devguide
15cc273 
Devguide PR: python/devguide#682
@encukou
Merge in the main branch
f37ec96
@encukou
Add a list of the stable ABI to the docs
d3c3107
@encukou
Add "Part of the Stable ABI" annotations to the documentation
179c132 
This replaces the existing :stableabi:, which wasn't really used.

Doc/data/stable_abi.dat is expanded to include all relevant
info (kind of the symbol and version it was added in), and the
c_annotations Sphinx extension reads this file and adds
notes similar to ones added by :stableabi: (which wasn't really
used).

Relevant prior work: python#23185
Co-authored-by: Will Ayd <william.ayd@icloud.com>
@encukou
Copy link
Member Author

encukou commented Apr 29, 2021

As per the PEP, I added a list of everything in the limited API, as well as notes in individual doc entries. A screenshot of the notes:

image

@encukou encukou marked this pull request as ready for review April 29, 2021 16:03
@encukou encukou mentioned this pull request Apr 29, 2021
Closed
@encukou
Copy link
Member Author

encukou commented May 6, 2021

The docs notes will now show platform restrictions, based on #ifdef, like the “on Windows" here:

image

orsenthil
orsenthil reviewed May 7, 2021
View reviewed changes
Copy link
Member

@orsenthil orsenthil left a comment

Choose a reason for hiding this comment

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

This looks good to me. I had few nits to clarify.

Doc/c-api/apiabiversion.rst Show resolved Hide resolved
Doc/c-api/apiabiversion.rst Outdated Show resolved Hide resolved
willingc
willingc approved these changes May 7, 2021
View reviewed changes
Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Thank you @encukou. Nice work improving clarity here. I've made a few suggestions for consideration. Thanks again ☀️

Doc/c-api/apiabiversion.rst Outdated

Thus ``3.4.1a2`` is hexversion ``0x030401a2``.
CPython exposes its version number in the following macros.
Note that these correspond to the version code is built with,
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps bold part of this sentence re: build version vs run-time version.

Doc/c-api/apiabiversion.rst Outdated
version information can be found by treating it as a 32 bit number in
the following manner:

+-------+-------------------------+-------------------------+
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice table as an example 😄

Doc/c-api/stable.rst Outdated
(x>=2) on the particular :ref:`platform <stable-abi-platform>`.

Defining ``Py_LIMITED_API`` to a value of :c:data:`PY_VERSION_HEX` will
do the same, but allow additional API added up to specified version,
Copy link
Contributor

Choose a reason for hiding this comment

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

Great improvements to this section. The wording in this item is slightly confusing. Does this mean: It will use the Limited API for this version and later versions. Earlier versions lose compatibility since additional API may exist prior to this version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the note! It really helps to have a second set of eyes here.
I've shuffled the paragraph around; hopefully things are clearer with this order.

Doc/c-api/stable.rst

For example, while :c:func:`PyList_GetItem` is available, its “unsafe” macro
variant :c:func:`PyList_GET_ITEM` is not.
The macro can be faster because it can rely on version-specific implementation
Copy link
Contributor

Choose a reason for hiding this comment

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

Good example.

@encukou
Copy link
Member Author

encukou commented May 7, 2021

Thank you for the thoughtful comments!

@miss-islington miss-islington merged commit b05955d into python:main May 11, 2021
@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

@miss-islington
Copy link
Contributor

Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-26034 is a backport of this pull request to the 3.10 branch.

@encukou encukou deleted the pep652-document branch May 11, 2021 14:04
@encukou
Copy link
Member Author

encukou commented May 11, 2021

This can be confusing; more and more often and I need a page to link to. So I merged this.
However, I'd be happy for more suggestions/improvements :)

willingc pushed a commit that referenced this pull request May 14, 2021
@miss-islington @encukou
bpo-43795: PEP 652 user documentation (GH-25668) (GH-26034)
3739371 
- Reformat the C API and ABI Versioning page (and extend/clarify a bit)
- Rewrite the stable ABI docs into a general text on C API Compatibility
- Add a list of Limited API contents, and notes for the individual items.
- Replace `Include/README.rst` with a link to a devguide page with the same info
(cherry picked from commit b05955d)

Co-authored-by: Petr Viktorin <encukou@gmail.com>

Co-authored-by: Petr Viktorin <encukou@gmail.com>
@pablogsal pablogsal mentioned this pull request Jul 18, 2021
Closed
@encukou encukou mentioned this pull request Jun 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orsenthil orsenthil orsenthil left review comments

@willingc willingc willingc approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@encukou @miss-islington @bedevere-bot @orsenthil @willingc @the-knights-who-say-ni