-
-
Notifications
You must be signed in to change notification settings - Fork 860
PEP 689: Add docs for unstable C API #1060
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.
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
Changes from 1 commit
b1aab58
73e2611
b210a10
b3b0a9b
d02a9e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,18 +4,20 @@ | |||||||||||||||||||||||||||||||||||||||||||||
Changing Python's C API | ||||||||||||||||||||||||||||||||||||||||||||||
======================= | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
The C API is divided into three sections: | ||||||||||||||||||||||||||||||||||||||||||||||
The C API is divided into these tiers: | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
1. The internal, private API, available with ``Py_BUILD_CORE`` defined. | ||||||||||||||||||||||||||||||||||||||||||||||
Ideally declared in ``Include/internal/``. Any API named with a leading | ||||||||||||||||||||||||||||||||||||||||||||||
underscore is also considered private. | ||||||||||||||||||||||||||||||||||||||||||||||
2. The public C API, available when ``Python.h`` is included normally. | ||||||||||||||||||||||||||||||||||||||||||||||
2. The “general” public C API, available when ``Python.h`` is included normally. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
Ideally declared in ``Include/cpython/``. | ||||||||||||||||||||||||||||||||||||||||||||||
3. The Limited API, available with ``Py_LIMITED_API`` defined. | ||||||||||||||||||||||||||||||||||||||||||||||
3. The Unstable C API, identified by the ``PyUnstable_`` name prefix. | ||||||||||||||||||||||||||||||||||||||||||||||
Ideally declared in ``Include/cpython/`` along with the general public API. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
4. The Limited C API, available with ``Py_LIMITED_API`` defined. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
Ideally declared directly under ``Include/``. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Each section has higher stability & maintenance requirements, and you will | ||||||||||||||||||||||||||||||||||||||||||||||
need to think about more issues when you add or change definitions in it. | ||||||||||||||||||||||||||||||||||||||||||||||
Each tier has different stability & maintenance requirements to think about | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
when you add or change definitions in it. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
The compatibility guarantees for public C API are explained in the | ||||||||||||||||||||||||||||||||||||||||||||||
user documentation, ``Doc/c-api/stable.rst`` (:ref:`python:stable`). | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -29,13 +31,18 @@ for building CPython itself, as indicated by a macro like ``Py_BUILD_CORE``. | |||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
While internal API can be changed at any time, it's still good to keep it | ||||||||||||||||||||||||||||||||||||||||||||||
stable: other API or other CPython developers may depend on it. | ||||||||||||||||||||||||||||||||||||||||||||||
For users, internal API is sometimes the best a workaround for a thorny problem | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
-- though those use cases should be discussed on `Discourse <https://discuss.python.org/c/30>`_ | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
or an issue so we can try to find a supported way to serve them. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
With PyAPI_FUNC or PyAPI_DATA | ||||||||||||||||||||||||||||||||||||||||||||||
----------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Functions or structures in ``Include/internal/`` defined with | ||||||||||||||||||||||||||||||||||||||||||||||
``PyAPI_FUNC`` or ``PyAPI_DATA`` are internal functions which are | ||||||||||||||||||||||||||||||||||||||||||||||
exposed only for specific use cases like debuggers and profilers. | ||||||||||||||||||||||||||||||||||||||||||||||
Ideally, these should be migrated to the :ref:`unstable-capi`. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
With the extern keyword | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -53,14 +60,18 @@ Private names | |||||||||||||||||||||||||||||||||||||||||||||
-------------- | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Any API named with a leading underscore is also considered internal. | ||||||||||||||||||||||||||||||||||||||||||||||
There are two main use cases for using such names rather than putting the | ||||||||||||||||||||||||||||||||||||||||||||||
definition in ``Include/internal/`` (or directly in a ``.c`` file): | ||||||||||||||||||||||||||||||||||||||||||||||
There is currently only one main use case for using such names rather than | ||||||||||||||||||||||||||||||||||||||||||||||
putting the definition in ``Include/internal/`` (or directly in a ``.c`` file): | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Internal helpers for other public API, which users should not call directly. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Note that historically, underscores were used for API that is better served by | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
the :ref:`unstable-capi`: | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* “provisional” API, included in a Python release to test real-world | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
usage of new API; | ||||||||||||||||||||||||||||||||||||||||||||||
* API for very specialized uses like JIT compilers. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Internal helpers for other public API; users should not use these directly; | ||||||||||||||||||||||||||||||||||||||||||||||
* “Provisional” API, included in a Python release to test real-world usage | ||||||||||||||||||||||||||||||||||||||||||||||
of new API. Such names should be renamed when stabilized; preferably with | ||||||||||||||||||||||||||||||||||||||||||||||
a macro aliasing the old name to the new one. | ||||||||||||||||||||||||||||||||||||||||||||||
See :pep:`"Finalizing the API" in PEP 590 <590#finalizing-the-api>` for an example. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Internal API Tests | ||||||||||||||||||||||||||||||||||||||||||||||
------------------ | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -134,6 +145,101 @@ When moving existing tests, feel free to replace ``TestError`` with | |||||||||||||||||||||||||||||||||||||||||||||
``PyExc_AssertionError`` unless actually testing custom exceptions. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
.. _unstable-capi: | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Unstable C API | ||||||||||||||||||||||||||||||||||||||||||||||
============== | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+149
to
+152
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm rather confused why this was added between Public C API and Limited C API, instead of between internal Internal API and Public C API. Isn't it specifically stated to be a step between the latter two, not the former two, so shouldn't it be moved there instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My reasoning is that the description builds on top of the public API section. If you're reading the whole document, the references would make you jump around so you'd end up with the order in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I see. I understand the rationale there, though IMO heading order still matters both for folks consulting specific sections/subsections (which are likely the majority of readers) and those reading the whole document sequentially, as it establishes a clear order and hierarchy of the API levels and the desired precedence for usage (limited, public, unstable, internal) rather confusingly implying a structure that doesn't reflect their actual conceptual organization, and requiring users to read the full text to ensure they know the correct order. In light of that, since the Limited API section doesn't really directly refer much to the Public C API section and is the highest-level tier (rather than the lowest, as the current order implies), what about instead switching it with the Internal API, which would make the order of the page from highest-level tier to lowest-level tier, keeping the Unstable API listed after Public C API as you have it now, but making a lot more sense overall. I.e.:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The suggested restructuring sounds plausible to me, but also feels like it would be better served by being done as its own PR rather than both adding new content and restructuring the existing content in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I brought it up here since it was this PR that introduced the regression on the ordering (it previously was ordered least to most portable, which IMO isn't perfect but much better than all over the place), and my initial suggestion was to just insert the category at the appropriate point in that order, i.e. above Public API. However, as @encukou brought up the added description of the unstable API substantially refers to that of the Public API, and as you mentioned this change would also be best made as a separate PR given, as I'd also noticed, it would conflate the two distinct changes (with squash-merge in use). Ideally, that change would have been considered and implemented first to avoid introducing a regression here and only fixing it later, but pragmatically speaking at this point I guess we just go ahead with this just fixing the tier list order (per the comment below) while deferring the section order to a followup PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds like a good order for the user-facing docs. Developers who implement the API generally go in the opposite direction. |
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
The unstable C API tier is meant for extensions that need tight integration | ||||||||||||||||||||||||||||||||||||||||||||||
with the interpreter, like debuggers and JIT compilers. | ||||||||||||||||||||||||||||||||||||||||||||||
Users of this tier may need to change their code with every minor release. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
In many ways, this tier is like the general C API: | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
- it's available when ``Python.h`` is included normally, | ||||||||||||||||||||||||||||||||||||||||||||||
- it should be defined in ``Include/cpython/``, | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
- it requires tests, so we don't break it unintentionally | ||||||||||||||||||||||||||||||||||||||||||||||
- it requires docs, so both we and the users | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
can agree on the expected behavior, | ||||||||||||||||||||||||||||||||||||||||||||||
- it is tested and documented in the same way. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
The differences are: | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
- Names of functions structs, macros, etc. start with the ``PyUnstable_`` | ||||||||||||||||||||||||||||||||||||||||||||||
prefix. This defines what's in the unstable tier. | ||||||||||||||||||||||||||||||||||||||||||||||
- The unstable API can change in minor versions, without any deprecation | ||||||||||||||||||||||||||||||||||||||||||||||
period. | ||||||||||||||||||||||||||||||||||||||||||||||
- A stability note is appears in the docs. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
This happens automatically, based on the name | ||||||||||||||||||||||||||||||||||||||||||||||
(via :cpy-file:`Doc/tools/extensions/c_annotations.py`). | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Despite being “unstable”, there are rules to make sure third-party code can | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
use this API reliably: | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Changes and removals can be done in minor releases (3.x.0, including | ||||||||||||||||||||||||||||||||||||||||||||||
Alphas and Betas for 3.x.0). | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
* Adding new unstable API *for an existing feature* is allowed even after | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
Beta feature freeze, up until the first Release Candidate. | ||||||||||||||||||||||||||||||||||||||||||||||
Consensus on Core Development Discourse or is needed in the Beta period. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
* Backwards-incompatible changes should make existing C callers fail to compile. | ||||||||||||||||||||||||||||||||||||||||||||||
For example, arguments should be added/removed, or a function should be | ||||||||||||||||||||||||||||||||||||||||||||||
renamed. | ||||||||||||||||||||||||||||||||||||||||||||||
* When moving API into or out of the Unstable tier, the old name | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
should continue to be available (but deprecated) until an incompatible | ||||||||||||||||||||||||||||||||||||||||||||||
change is made. In other words, while we're allowed to break calling code, | ||||||||||||||||||||||||||||||||||||||||||||||
we shouldn't break it *unnecessarily*. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Moving API from the public tier to unstable | ||||||||||||||||||||||||||||||||||||||||||||||
------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Expose the API under its new name, with the ``PyUnstable_`` prefix. | ||||||||||||||||||||||||||||||||||||||||||||||
* Make the old name an alias (e.g. a ``static inline`` function calling the | ||||||||||||||||||||||||||||||||||||||||||||||
new function). | ||||||||||||||||||||||||||||||||||||||||||||||
* Deprecate the old name, typically using ``Py_DEPRECATED``. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
* Announce the change in What's New. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
The old name should continue to be available until an incompatible change is | ||||||||||||||||||||||||||||||||||||||||||||||
made. Per Python’s backwards compatibility policy (:pep:`387`), | ||||||||||||||||||||||||||||||||||||||||||||||
this deprecation needs to last at least two releases (modulo SC exceptions). | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
The rules are relaxed for API that was introduced before the official | ||||||||||||||||||||||||||||||||||||||||||||||
Unstable tier. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
For the following, you can make an incompatible change (and remove the old name) | ||||||||||||||||||||||||||||||||||||||||||||||
as if the function was already part of the Unstable tier: | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* API introduced before Python 3.12 that is documented to be less stable than default. | ||||||||||||||||||||||||||||||||||||||||||||||
* API introduced before Python 3.12 that was named with a leading underscore. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Moving API from the private tier to unstable | ||||||||||||||||||||||||||||||||||||||||||||||
-------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Expose the API under its new name, with the ``PyUnstable_`` prefix. | ||||||||||||||||||||||||||||||||||||||||||||||
* If the old name is documented, or widely used externally, | ||||||||||||||||||||||||||||||||||||||||||||||
make it an alias and deprecate it (typically with ``Py_DEPRECATED``). | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
It should continue to be available until an incompatible change is made, | ||||||||||||||||||||||||||||||||||||||||||||||
as if it was previously public. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
This applies even to underscored names. Python wasn't always strict with | ||||||||||||||||||||||||||||||||||||||||||||||
the leading underscore. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
* Announce the change in What's New. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Moving API from unstable to public | ||||||||||||||||||||||||||||||||||||||||||||||
---------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
* Expose the API under its new name, without the ``PyUnstable_`` prefix. | ||||||||||||||||||||||||||||||||||||||||||||||
* Make the old ``PyUnstable_*`` name be an alias (e.g. a ``static inline`` | ||||||||||||||||||||||||||||||||||||||||||||||
function calling the new function). | ||||||||||||||||||||||||||||||||||||||||||||||
* Announce the change in What's New. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
The old name should continue to be available until an incompatible change is | ||||||||||||||||||||||||||||||||||||||||||||||
made. | ||||||||||||||||||||||||||||||||||||||||||||||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no subject matter expert opinion on the changes, of course (though ISTM that removing the alias after a backward-incompatible change to a public API should still be at least mentioned, as it is safer than silently changing behavior) but if you want to accept it, I've made @ncoghlan 's comment from the above into an actual suggestion, on the correct lines, and with some copyediting cleanup to make it more concise and avoid repetition.
Suggested change
Or, with an explicit mention of removal after an incompatible change, as before:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a public API, so incompatible changes are not allowed. This new text should not suggest that they are. If we want to make an incompatible change to a public API, we have to add a new API and deprecate the old one instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I see, thanks—as mentioned, my C-API expertise is non-existent. For my future reference, where's that documented again? I don't see where that is explicitly mentioned in the C API stability policy, PEP 387 or this document, which are the places I would expect it to be. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @CAM-Gerlach It's basically the entirety of the "Making incompatible changes" section of PEP 387. (Particularly steps 2 and 3.)
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, its certainly not "the entirety of the section"; it's the very specific point above I'm after. On careful inspection I do see what specifically implies this, though it is a bit buried and took multiple reads to find:
The language here really should be more clear and explicit, as the "may"s and "should"s used here certainly don't send nearly the same message as "incompatible changes are not allowed" and "If we want to make an incompatible change to a public API, we have to add a new API and deprecate the old one instead." Also, "old usage" is somehwt unclear terminology. For that, it should instead state something like
To note, that isn't always true at least for Python APIs; I can recall several instances (e.g. regex group identifiers) where incompatible changes were made to existing functions for various reasons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I think the alias should be removed as soon as the public API is deprecated. If you signed up for updating your code to react to implementation changes, you should update it when the API is no longer favoured.
Suggested change
This is technically a change in the PEP, I'll need to run it by the SC. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
(I plan to do this after merging, and revert if necessary) |
||||||||||||||||||||||||||||||||||||||||||||||
There's no need to deprecate it (it was unstable to begin with), | ||||||||||||||||||||||||||||||||||||||||||||||
but there's also no need to break working code just because some function | ||||||||||||||||||||||||||||||||||||||||||||||
is now ready for a wider audience. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
Limited API | ||||||||||||||||||||||||||||||||||||||||||||||
=========== | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment with the order here, only more crucial—"tiers" explicitly denotes an ordered hierarchy, and yet internal API is listed and numbered first, then public API, then unstable API, then Limited API, which directly implies that this is the order of the tiers. Swapping "internal" and "limited" would fix this (as would swapping "general" and "unstable", as I originally suggested, but wouldn't match the order in the document, and going from highest-level/most recommended to lowest-level/least recommended makes more sense anyway).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the ordering issue is real, but I think trying to resolve it directly in this PR will make it harder to review the content changes.
My suggestion would be to instead file a docs enhancement request to restructure the API tier docs to run from either most-to-least or least-to-most portable, rewording the individual sections as necessary.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, while is this PR that is introducing a regression (as the order currently is least-to-most portable), practically speaking the approach you suggest is the most straightforward to implement at this point.
That being said, we should at least try to avoid introducing too much of an unnecessary regression here, particularly in this list where the regression is more serious but much easier to fix. Therefore, to help mitigate this while minimizing the further changes here, the simplest fix is to just add the unstable tier to its correct place in this explicitly ordered tier list per the current least-to-most portable order (while keeping the document section order as it is now in this PR).
That greatly reduces the potential confusion while not requiring any additional changes, and makes much more sense than trying to match the section order since this is an ordered list of API tiers, not sections on the page (that's what the ToC is for, and the page section order not being as important is the justification for the regression in the first place).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds reasonable, thanks.
FWIW, the original order matches an ideal of how APIs are “promoted” as they mature: first implement the functionality, then make it usable for others, then make sure it's rock-solid.
In this list, the unstable API is on the same level as the general public API. And it's a niche enough topic to be documented with “same as that except for the following”.