Fix lookup table enhancement removing transparency (NaNs)#3377
Fix lookup table enhancement removing transparency (NaNs)#3377mraspaud merged 5 commits intopytroll:mainfrom
Conversation
Preserve and replace NaN values lost during lookup. For image transparency.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3377 +/- ##
=======================================
Coverage 96.33% 96.34%
=======================================
Files 466 466
Lines 59119 59160 +41
=======================================
+ Hits 56953 56995 +42
+ Misses 2166 2165 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| lut = luts[:, index] if len(luts.shape) == 2 else luts | ||
| band_data = band_data.clip(0, lut.size - 1) | ||
| # Convert to uint8, with NaN/null values changed into 0 | ||
| band_data = np.nan_to_num(band_data).astype(np.uint8) |
There was a problem hiding this comment.
Your changes are similar to the lines here that were added a while ago. This was done in #3315 by @avalentino to avoid warnings on NaN data and an error on one particular architecture. Your new solution should be fine to handle this as well if re-implemented here. My main question is, is this something we want the user to be able to control? Mainly, should the user be able to have NaN map to a value in the LUT? Technically that's what the nan_to_num is allowing but the proposed solution is ensuring that NaNs always remain NaNs.
Regardless, the proposed solution seems like something that could be handled inside this _lookup_table function and would therefore have better performance as it happens in parallel per-chunk of the dask array. With your code moved into this function or rather re-implemented with pure-numpy functions/masking/indexing then this nan_to_num line could probably be removed.
There was a problem hiding this comment.
I agree that it would be better to have it in _lookup_table instead, so I'll move it/re-implement it there.
Regarding your question about the user mapping NaN to a (custom?) value in the LUT, after speaking about it with @mraspaud, it could be useful but I'm not so sure if it's needed, but in any case it's a bit outside of the scope of this pull request. It would also be a bit more complicated to implement and would change the functionality a bit too much (atleast for right here, right now).
There was a problem hiding this comment.
I've rewritten this comment at least 3 times now because I kept coming up with a different way of implementing this, but I think I've decided none of those ways were better than what you have here. So 👍
Neened to keep nan_to_num to avoid warning: "RuntimeWarning: invalid value encountered in cast"
djhoese
changed the title
3 participants
In colormap.py in Enhancements, preserves the NaN values lost during lookup and replaces them again where they were lost. For SMHI only relevant for snow_age.
AUTHORS.mdif not there already