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-115653: Add docs for the PyCode_GetFirstFree and correct return type for the PyCode_GetNumFree#115654
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
Uh oh!
There was an error while loading.Please reload this page.
wrongnull commentedFeb 20, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
BTW, should |
Doc/c-api/code.rst Outdated
| Return the number of free variables in *co*. | ||
| .. c:function:: int PyCode_GetFirstFree(PyCodeObject *co) | ||
| Return the number of local + cell variables in *co*. |
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.
Document what the API does, not its implementation details:PyCode_GetFirstFree returns the position (or offset) of the first free variable inco.
FTR:@markshannon and@iritkatriel introduced this API ingh-100721. It is available via Python.h.
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. But my question above still stands. If I'm right I might make a separate pull request for this.
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.
See the PR I linked to, especially#100721 (comment) and49ca044.
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.
Thank you. If there is anything else here that I could fix, I'll be happy to do 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.
I would still like to hear from either@markshannon or@iritkatriel; I'm not sure this was intended as a public API.
markshannonFeb 21, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
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.
It was and it wasn't. We don't really want to expose it, but if we don't then Cython and other C extensions will access the internal fields directly, which is worse.
Now that we have an unstable API, it should probably be part of that.
I'd like all code object C APIs to be unstable, but it's probably too late for that.
Maybe rename it toPyUnstableCode_GetFirstFree?
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.
Maybe rename it to PyUnstableCode_GetFirstFree?
Sounds like a good idea, but since it is already included in 3.12, we have to deprecatePyCode_GetFirstFree andPyUnstableCode_GetFirstFree in 3.13.
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.
Follow-up issue:#115756.
In general, please split up multiple changes in atomic PRs; the distinct changes might not have equal backport requirements. For these two changes it looks to me both of them are applicable for a 3.12 backport only, so you're fine. |
Doc/c-api/code.rst Outdated
| @@ -30,10 +30,14 @@ bound into a function. | |||
| Return true if *co* is a :ref:`code object <code-objects>`. | |||
| This function always succeeds. | |||
| .. c:function::int PyCode_GetNumFree(PyCodeObject *co) | |||
| .. c:function::Py_ssize_t PyCode_GetNumFree(PyCodeObject *co) | |||
| Return the number of free variables in *co*. | |||
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.
| Return the number of free variables in*co*. | |
| Return the number of free variables ina code object. |
Doc/c-api/code.rst Outdated
| Return the number of free variables in *co*. | ||
| .. c:function:: int PyCode_GetFirstFree(PyCodeObject *co) | ||
| Return the position of first free variable in *co*. |
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.
| Return the position of first free variable in*co*. | |
| Return the position ofthefirst free variable ina code object. |
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.
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.
LGTM.
Thanks@wrongnull for the PR, and@vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Correct the return type of the PyCode_GetNumFree() documentation.(cherry picked from commit10fc467)Co-authored-by: Bogdan Romanyuk <65823030+wrongnull@users.noreply.github.com>
GH-115752 is a backport of this pull request to the3.12 branch. |
| Return the number of free variables in *co*. | ||
| Return the number of free variables in a code object. |
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.
@vstinner, I think this is the suggested style:
| Return the number of free variables inacode object. | |
| Return the number of free variables in code object *co*. |
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.
Ah, I just merged the change. Is it worth it to change the style, or is it ok to merge PRgh-115752 backport? I let you decide :-)
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.
No, it's not worth it to change the style, but I think we should try to keep it in mind for next time ;)
Uh oh!
There was an error while loading.Please reload this page.
Correct the return type of the PyCode_GetNumFree() documentation.
Correct the return type of the PyCode_GetNumFree() documentation.
Correct the return type of the PyCode_GetNumFree() documentation.
Uh oh!
There was an error while loading.Please reload this page.
📚 Documentation preview 📚:https://cpython-previews--115654.org.readthedocs.build/