gh-115756: make PyCode_GetFirstFree an unstable API by wrongnull · Pull Request #115781 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation10 Commits7 Checks0 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
Contributor
•
edited by bedevere-appbot
Loading
According to this , should I mention it in the documentation in a separate pull request?
No; this PR represents an atomic change. Documentation (and preferably tests) follow. Your previous PR consisted of two different changes1.
But you cannot just remove the existing API; you need to keep it and deprecate it (see PEP-387).
Footnotes
- 1: fix the return type of one API. 2: change the documentation of another API. ↩
Here's an example of deprecation: #105397
The question may seem silly but should I add a news section?
| @@ -226,11 +226,15 @@ static inline Py_ssize_t PyCode_GetNumFree(PyCodeObject *op) { |
|---|
| return op->co_nfreevars; |
| } |
| static inline int PyCode_GetFirstFree(PyCodeObject *op) { |
| static inline int PyUnstable_Code_GetFirstFree(PyCodeObject *op) { |
| assert(PyCode_Check(op)); |
| return op->co_nlocalsplus - op->co_nfreevars; |
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the deprecation has no detail, for me, it can be removed as soon as in Python 3.15. But the doc is more specific:
"The old name is deprecated, but will remain available until the signature changes again."
Please add a comment to explain it here.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the deprecation has no detail, for me, it can be removed in Python 3.15.
What are you basing this on?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP 387: Similarly a feature cannot be removed without notice between any two consecutive releases.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, I didn't disable automerge after your comment. I didn't mean to ignore you :(
But IMO, the “Basic Policy” is just a summary. The “Making Incompatible Changes” section has more detailed instructions.
If you disagree with my reading, let's take it to Discourse; this isn't a good place for it.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just that this function has a specific comment in the doc, IMO it's worth it to repeat it in the header to avoid future eagger removal. But well, now that the change is merged, it's maybe no longer worth it.
vstinner pushed a commit to vstinner/cpython that referenced this pull request
adorilson pushed a commit to adorilson/cpython that referenced this pull request
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request
Reviewers
