Fix treatment of 'BLANK' and ignore_blank for uncompressed integer images#16636
Fix treatment of 'BLANK' and ignore_blank for uncompressed integer images#16636dhomeier wants to merge 3 commits into
ignore_blank for uncompressed integer images#16636Conversation
|
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.
|
|
👋 Thank you for your draft pull request! Do you know that you can use |
dhomeier
force-pushed
the
uncompressed-blanks
branch
2 times, most recently
from
28a530b to
f132a93
Compare
dhomeier
force-pushed
the
uncompressed-blanks
branch
from
f132a93 to
9e58bb0
Compare
| raw_data = data | ||
| data = None | ||
| self._uint = False # Reset this to enable conversion to float | ||
| self._blank += self._orig_bzero # Same scaling to uint as for data |
There was a problem hiding this comment.
Hi @dhomeier,
Sorry it took me too long to come to this PR but I was focused on the compression refactor :)
I would prefer to avoid the few lines above which are making this function harder to understand (tweaking the hdu to make it behaves as integer data). I think we can handle directly the blank case for unsigned integer in _convert_pseudo_integer, something like this:
--- astropy/io/fits/hdu/image.py
+++ astropy/io/fits/hdu/image.py
@@ -795,6 +795,11 @@ class _ImageBaseHDU(_ValidHDU):
data = np.array(data, dtype=dtype)
data -= np.uint64(1 << (bits - 1))
+ if self._blank is not None:
+ data = np.array(data, dtype=np.float32)
+ blanks = data.flat == self._blank + self._orig_bzero
+ data.flat[blanks] = np.nan
+
return data
def _get_scaled_image_data(self, offset, shape):
@@ -827,11 +832,6 @@ class _ImageBaseHDU(_ValidHDU):
data = None
if not (self._orig_bzero == 0 and self._orig_bscale == 1):
data = self._convert_pseudo_integer(raw_data)
- if not (self._blank is None or data is None or data.dtype.kind != "u"):
- raw_data = data
- data = None
- self._uint = False # Reset this to enable conversion to float
- self._blank += self._orig_bzero # Same scaling to uint as for data
Does is it reasonable ?
|
Hi humans 👋 - this pull request hasn't had any new commits for approximately 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then. In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary. If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock. If you believe I commented on this pull request incorrectly, please report this here. |
|
@dhomeier - ping, can you rebase (and have a look at my comment above if you missed it) ? |
astrofrog
added
backport-v7.2.x
5 participants
Description
Discussion of appropriate treatment of
BLANKvalues in compressed image HDUs in #16550 revealed that in the uncompressed case integer data with the'BLANK'keyword present are not handled consistently either, being converted tofloatwith the invalid pixels set toNaNfor signed integers, but left unmodified in the case of (unsigned) "scaled" integers. On updating the tests I also found that in the former case, theignore_blank=Trueparameter setting for leaving the integer data unmodified is only honoured forPrimaryHDU, but silently ignored for images in anExtensionHDU. I could not find any rationale for this other thanImageHDUhaving been missed when this kwarg was introduced in #2307.There was one suggestion that these operations should be set on a per-HDU basis, but since the arguments are passed at the
fits.openlevel, this would be non-straightforward to implement.This PR therefore leaves all integer-type HDU data unmodified for
ignore_blank=True, and converts them to float otherwise, if a valid 'BLANK' is set in the header (as per the documentation, and as implemented for compressed HDUs in #16550).Addresses #7789 insofar as conversion can now be reliably avoided with
ignore_blank.Should be reviewed and pulled in as appropriate before merging #15474 to ensure that blank treatment stays consistent in
CompImageHDUas well (which does not have anignore_blankoption yet / in 6.1.x).