DOC: document caveats of ndarray.resize on 3.14 and newer#31021
Conversation
|
Marking as a draft while I consider Sam's suggestion: #30991 (comment) |
mattip
left a comment
There was a problem hiding this comment.
Thanks. I think this is useful even if it turns out we can work around CPython refcounting fragility. We may need to adjust the post-3.13 comment depending on the outcome of the other work.
| reference to be resized, because it is impossible to differentiate between | ||
| an array with one reference created via an extension and a uniquely | ||
| referenced array defined in a Python function. On Python 3.14 and newer, | ||
| the array must be uniquely referenced. |
There was a problem hiding this comment.
I find this a bit confusing. Maybe "On Python 3.13 and older, the check allows objects with exactly one reference to be reallocated in-place. On Python 3.14 and newer, the array must be uniquely referenced, which is getting progressively more complicated to detect.
| * On Python 3.13 and older, the check allows objects with exactly one | ||
| * reference to be resized, because it is impossible to differentiate between | ||
| * an array with one reference created via an extension and a uniquely | ||
| * referenced array defined in a Python function. |
There was a problem hiding this comment.
Maybe like in the docs
On Python 3.13 and older, the check allows objects with exactly one reference to be reallocate in-place. On newer versions ...
Co-authored-by: Matti Picus <matti.picus@gmail.com>
Co-authored-by: Matti Picus <matti.picus@gmail.com>
|
I think I responded to all your comments @mattip |
mhvk
left a comment
There was a problem hiding this comment.
Yes, I think documenting behaviour is preferred over trying to make .resize work magically. Some small comments in-line.
| * must be uniquely referenced. In some cases this can lead to spurious | ||
| * ValueErrors on Python 3.14. | ||
| * | ||
| * On Python 3.13 and older, the check allows objects with exactly one |
There was a problem hiding this comment.
Strange partial duplication with text above. Merge?
|
I think I responded to all review comments. |
mattip
left a comment
There was a problem hiding this comment.
On tiny nit, but can be merged without it.
Co-authored-by: Matti Picus <matti.picus@gmail.com>
|
Thanks Nathan. |
Closes #30991.
This doesn't "fix" the regression, because I don't think there's a way to do that without introducing new bugs or depending on interpreter internals and/or trying to argue for yet another
PyUnstableC API function we can use.I'm also a little concerned that the old check is actually incorrect for arrays created by C extensions: the check on 3.13 and older allows resizing an array created directly via e.g.
PyArray_FromAnywith exactly one reference.IMO it's better to keep the logic of the check the same and simply improve the documentation and error messages.
Now when the reference count is exactly 2, the error message on 3.14 and newer explains that this might be a false positive.
I added tests which trigger the behaviors we expect to happen on different Python versions. I also deleted some out-of-date docs about an unused
orderargument and added some notes that resizing in-place can lead to memory fragmentation and worse overall memory usage.Also looking for any ideas for ways to make the error messages and docstrings less wordy.