Uh oh!
There was an error while loading. Please reload this page.
gh-105927: Add PyWeakref_GetRef() function - #105932
Conversation
vstinner
commented
Jun 20, 2023
Once this PR will be merged, I will create a follow PR to replace PyWeakref_GET_OBJECT() with PyWeakref_GetRef(). I have it locally, but GitHub doesn't let me create a "patch serie". |
Uh oh!
There was an error while loading. Please reload this page.
In new API, please don't return NULL without an exception set. |
vstinner
commented
Jun 20, 2023
It doesn't return NULL with an exception set. |
* Add tests on PyWeakref_NewRef(), PyWeakref_GetObject(), PyWeakref_GET_OBJECT() and PyWeakref_GetRef(). * PyWeakref_GetObject() now raises a TypeError if the argument is not a weak reference, instead of SystemError.
vstinner
commented
Jun 20, 2023
I changed the API to: I also added tests. @erlend-aasland@encukou: Please review the updated PR. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
erlend-aasland
commented
Jun 20, 2023
Using an |
* Change Sphinx formatting * Don't change PyWeakref_GetObject() exception
vstinner
commented
Jun 20, 2023
@erlend-aasland: I addressed your review. |
erlend-aasland
commented
Jun 20, 2023
Thanks! Looks good. |
erlend-aasland
left a comment
There was a problem hiding this comment.
LGTM; I'm interested in Petr's review, though :)
Enhance also the doc: specific strong/borrowed reference
erlend-aasland
commented
Jun 20, 2023
2de3897 is in itself a nice docs update that could be backported through 3.11! :) |
Uh oh!
There was an error while loading. Please reload this page.
vstinner
commented
Jun 21, 2023
I dislike "3 states" return value: error, case 1 or case 2. It forces to store the return value in a variable and uses 2 if to cover the 3 code paths. Also, IMO the API is more error prone since someone may write the wrong test for the error case. Well, I don't know how much is just m personal taste and what is objective here 😁 I prefer to only return -1 on error, and 0 for the other cases. So I don't need an extra variable. |
📚 Documentation preview 📚: https://cpython-previews--105932.org.readthedocs.build/