Repository navigation
Fix uint conversion after slicing - #8982
Conversation
Fix astropy#5490. The _uint attribute was not transfered to the sliced array, so the data was later converted either correctly (if it was converted first before slicing) or as float in the other case.
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
Looks good! Thanks for figuring out this very old issue!
Since my comments are really just queries, approving now.
|
|
||
| # We got a view; change it back to our class, and add stuff | ||
| out = out.view(type(self)) | ||
| out._uint = self._uint |
There was a problem hiding this comment.
I vaguely recall adding some of the above code, but looking at it again I wonder a bit why... Normally, an ndarray subclass is supposed to do this in __array_finalize__, but here of course the view as a recarray and then the view back here circumvent that (and from a quick look, just doing out.__arrray_finalize__(self) would seem unlikely to do quite the right thing)
Anyway, I think the fix here is good, though perhaps have a quick look at __array_finalize__ and see if anything else is missing (since _uint at least is updated there).
There was a problem hiding this comment.
Yes, I should have mentioned that I requested your review because you changed a bit this code in the past (e.g. #6955), but the issue was already there before. The problem is that with the recarray view we loose the FITS_rec attributes, so they are no more available in __array_finalize__ when viewing back to FITS_rec.
I'm not completely sure about the other attributes, I think that the missing ones can be recomputed from the coldefs.
| assert_array_equal(data[sel]['b'], dataref[sel]) | ||
| assert data[sel]['b'].dtype == dataref[sel].dtype | ||
|
|
||
| with fits.open(self.temp('test_scaled_slicing.fits')) as hdulist: |
There was a problem hiding this comment.
Are we opening the file twice to prevent being fooled by a cache or so?
There was a problem hiding this comment.
Yes somewhat, in the first case as data['b'] is accessed the data is converted and cached, so before the fix data[sel] had the wrong value for _uint but was still able to print correct values because the cache is copied.
|
Thanks for the review @mhvk ! |
|
And thanks for the fix, and for explaining - all makes sense. (Though I think we should be able to be more properly |
Fix uint conversion after slicing
Fix uint conversion after slicing
Fix #5490.
The _uint attribute was not transfered to the sliced array, so the data
was later converted either correctly (if it was converted first before
slicing) or as float in the other case.