-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
gh-141004: Document soft-deprecated symbols #141634
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
base: main
Are you sure you want to change the base?
gh-141004: Document soft-deprecated symbols #141634
Conversation
Yes. My opinion is that documentation for deprecated API should tell you what to do if you find it in some dusty legacy codebase. |
Ok, I can do that. I have a few ideas on how we could format this -- which do you prefer?
|
|
I'm not sure that a "Soft-deprecated" section is needed.
That sounds like a reasonable approach. |
I'd prefer “Deprecated API” sections at the end of pages. Alas, that might not be a popular preference :(
That makes sense for “close friends” -- e.g. if the porting instructions tell you to use a specific other function instead. |
If others are fine with a Deprecated API section, I would also be fine with it. |
|
See https://docs.python.org/3/c-api/unicode.html#deprecated-api for an example. I think it's working pretty well. |
It looks nice. |
|
Updated, let me know what you think.
In practice, we don't have great replacements for all of these, so I just documented some of them as "do not use". For example, what is a user supposed to do if they find |
| This is a :term:`soft deprecated` constant representing the size of a | ||
| preallocated table inside :c:type:`PySetObject` instances. | ||
| This is documented solely for completeness and not useful to users in any | ||
| way. If looking for the size of a set, use :c:func:`PySet_Size` instead. |
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.
what is a user supposed to do if they find PySet_MINSIZE somewhere?
This is indeed not obvious. All the more reason for the docs to mention it, if we want to add docs.
What about something like:
| This is a :term:`soft deprecated` constant representing the size of a | |
| preallocated table inside :c:type:`PySetObject` instances. | |
| This is documented solely for completeness and not useful to users in any | |
| way. If looking for the size of a set, use :c:func:`PySet_Size` instead. | |
| A :term:`soft deprecated` constant representing the size of an internal | |
| preallocated table inside :c:type:`PySetObject` instances. | |
| This is documented solely for completeness, as there are no guarantees | |
| that a given version of CPython uses preallocated tables with a fixed | |
| size. | |
| In code that does not deal with unstable set internals, | |
| :c:macro:`!PySet_MINSIZE` can be replaced with a small constant like ``8``. | |
| If looking for the size of a set, use :c:func:`PySet_Size` instead. | |
| .. deprecated:: next | |
| The constant is :term:`soft deprecated`. |
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'm not too fond of using .. deprecated for this, because the next (or whatever version we put in there) would imply that it's fine to use for earlier versions. We don't want people using this on any version.
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's fair.
On the other hand, without .. deprecated, automated tools won't be able to tell that these are deprecated.
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.
Unfortunately, I don't think our documentation is sufficient for automated tools yet. (For example, we document a bunch of macros as functions, which would break if used to generate bindings.) I do agree that we should eventually get C API documentation to that point, but that's a project I plan to work on once #141004 is done.
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 still think .. deprecated is better in the mean time. But I won't block the PR on that :)
Doc/c-api/long.rst
Outdated
| These macros are :term:`soft deprecated`. They represent internal constants | ||
| for :c:type:`PyLongObject` instances. | ||
| Do not use these; use :c:func:`PyLong_GetNativeLayout` instead. |
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.
Here I'd recommend listing the equivalent expressions, to make these easier to replace.
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.
Done.
Doc/c-api/long.rst
Outdated
| .. c:macro:: PyLong_SHIFT | ||
| This is currently equivalent to ``30``. |
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.
It can be 15, see the configure option --enable-big-digits.
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.
Fixed via Petr's suggestion.
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
These are all APIs that expose implementation details. Instead of truly documenting them and what they do, this PR just adds a note saying "don't use this" with no additional information. This is a good compromise for us; if someone saw these in their IDE, Python will still provide some form of documentation for them, but they won't lead to any additional maintenance burden.
cc @vstinner @encukou -- I suspect both of you will have some strong opinions here.
📚 Documentation preview 📚: https://cpython-previews--141634.org.readthedocs.build