Repository navigation
Fix access to VLA data after slicing - #16996
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
| out._heapoffset = self._heapoffset | ||
| out._heapsize = self._heapsize | ||
| out._tbsize = self._tbsize | ||
| out._gap = self._gap |
There was a problem hiding this comment.
@mhvk - I never really understood Numpy subclassing but, isn't there a better way to propagate those custom attributes ?
There was a problem hiding this comment.
Normally, this gets done automatically by __getitem__, which by default would call result.__array_finalize__(self), but here on l.516 we first specifically view as a recarray (for reasons commented on but which I don't know couldn't be worked around another way), and therefore one gets result.__array_finalize(self.view(np.recarray)), i.e., the wrong finalize. So, one could avoid the duplication by explicitly doing out.__array_finalize__(self) here (note that I didn't check whether that does exactly the same thing as the code here).
| the stated criteria can be found. | ||
| """ | ||
| raw_data_bytes = self.nbytes + self._heapsize | ||
| raw_data_bytes = self._tbsize + self._heapsize |
There was a problem hiding this comment.
Main fix is here, with the propagation of heap size and the original array size.
|
Seems to create problems with other tests, was a bit optimistic when running only VLA tests 😬 . |
|
I fixed the issue so tests are now passing, ready to review :) |
| assert arr.tolist() == [[45, 56]] | ||
| assert arr[0].dtype == np.int16 | ||
|
|
||
| def test_vla_slice2(self): |
There was a problem hiding this comment.
couldn't these test be merged into a single parametrized function ?
There was a problem hiding this comment.
This would make things harder to read and understand. If there were more than 2 maybe but here simple is better imo.
|
Thanks, all! |
…996-on-v6.1.x Backport PR #16996 on branch v6.1.x (Fix access to VLA data after slicing)
Fix #16990
Description
This pull request is to address ...
Fixes #