Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
gh-115756: make PyCode_GetFirstFree an unstable API#115781
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
No; this PR represents an atomic change. Documentation (and preferably tests) follow. Your previous PR consisted oftwo different changes1. But you cannot just remove the existing API; you need to keep it and deprecate it (seePEP-387). Footnotes
|
Here's an example of deprecation:#105397 |
The question may seem silly but should I add a news section? |
Yes, you should. it's a user-visible change. |
Uh oh!
There was an error while loading.Please reload this page.
| @@ -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; | |||
| } | |||
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.
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.
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.
If the deprecation has no detail, for me, it can be removed in Python 3.15.
What are you basing this on?
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.
PEP 387: Similarly a feature cannot be removed without notice between any two consecutive releases.
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 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.
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'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.
Uh oh!
There was an error while loading.Please reload this page.
According tothis , should I mention it in the documentation in a separate pull request?