Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-114329: AddPyList_GetItemRef function#114504
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
The new `PyList_GetItemRef` is similar to `PyList_GetItem`, but returnsa strong reference instead of a borrowed reference. Additionally, if thepassed "list" object is not a list, the function sets a `TypeError`instead of calling `PyErr_BadInternalCall()`.
When I proposed such change,@serhiy-storchaka asked me to stick to |
Doc/c-api/list.rst Outdated
| :exc:`TypeError` exception. | ||
| This behaves like :c:func:`PyList_GetItem`, but returns a | ||
| :term:`strong reference` instead of a :term:`borrowed reference`. |
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 suggest to make this function the new reference, and documentPyList_GetItem() as: "Similar to PyList_GetItemRef(), but return a borrowed reference".
Lib/test/test_capi/test_list.py Outdated
| @@ -112,6 +112,15 @@ def test_list_get_item(self): | |||
| # CRASHES get_item(21, 2) | |||
| # CRASHES get_item(NULL, 1) | |||
| def test_list_get_item_ref(self): | |||
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.
Can you share most code between test_list_get_item() and test_list_get_item_ref()?
Uh oh!
There was an error while loading.Please reload this page.
Misc/stable_abi.toml Outdated
| [function.PyList_GetItemRef] | ||
| added = '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.
Move it to the end of the file.
Is it RuntimeError or SystemError? The only discussed difference was returning a strong reference instead of a weak reference. If we want to change also the exception type, it should be a separate discussion. Most concrete type C API raises SystemError if they are called with wrong type. It is considered a programming error, not a data error. |
I suggest to document that the function "parameter must be list" , without mentioning the exact exception type. |
The |
I've removed the sentence mentioning |
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.
The TypeError behavior is explicitly stated in the C API WGcapi-workgroup/decisions#9 (comment). See alsocapi-workgroup/api-evolution#29 (comment) andPyWeakref_GetRef().
Oh ok, I didn't notice that TypeError was explicitly mentioned.
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.
LGTM too.
I will merge the PR if@serhiy-storchaka approve.
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 WG does discuss the exception type, it'll take some time.
Let's merge this, the type can be changed later
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
The new `PyList_GetItemRef` is similar to `PyList_GetItem`, but returnsa strong reference instead of a borrowed reference. Additionally, if thepassed "list" object is not a list, the function sets a `TypeError`instead of calling `PyErr_BadInternalCall()`.
The new `PyList_GetItemRef` is similar to `PyList_GetItem`, but returnsa strong reference instead of a borrowed reference. Additionally, if thepassed "list" object is not a list, the function sets a `TypeError`instead of calling `PyErr_BadInternalCall()`.
Uh oh!
There was an error while loading.Please reload this page.
The new
PyList_GetItemRefis similar toPyList_GetItem, but returns a strong reference instead of a borrowed reference. Additionally, if the passed "list" object is not a list, the function sets aTypeErrorinstead of callingPyErr_BadInternalCall().PyList_GetItemRef, a variant ofPyList_GetItemthat returns a strong reference #114329📚 Documentation preview 📚:https://cpython-previews--114504.org.readthedocs.build/