Fix the ninjotiff writer to provide correct scale and offset #889
Conversation
Codecov Report
@@ Coverage Diff @@
## master #889 +/- ##
==========================================
+ Coverage 85.28% 85.36% +0.07%
==========================================
Files 174 174
Lines 25822 26023 +201
==========================================
+ Hits 22022 22214 +192
- Misses 3800 3809 +9
Continue to review full report at Codecov.
|
satpy/composites/__init__.py
Outdated
| new_attrs["wavelength"] = None | ||
| new_attrs.pop("units", None) | ||
| new_attrs.pop('calibration', None) | ||
| new_attrs.pop('modifiers', None) |
There was a problem hiding this comment.
Doing this is risky. There are things like the DNB enhancements or any other single band composite where accidentally keeping any one of these would be incorrect. Especially wavelength. Yes the composite could/should be removing these but I think we've been depending on this being part of the base compositor. I could also see this being separated where BaseCompositor has all of the above functionality, but GenericCompositor (being made for images - L, RGB, etc) could remove these attributes because they don't make sense.
What composites do you have that you need to keep this information?
There was a problem hiding this comment.
IR. It is important that we keep the units so that it can be converted to the right unit later.
There was a problem hiding this comment.
Let me check if I can revert this
There was a problem hiding this comment.
What "IR"? If it is using GenericCompositor then it is an image and should be considered unitless, right? The subclass compositor could also re-add units although I'm not sure that is great either.
There was a problem hiding this comment.
That's the thing with tiffs like these: the values in the file form an image be must be revertable to physical quantities
There was a problem hiding this comment.
so it's both an image and a physical dataset
satpy/composites/__init__.py
Outdated
| num = len(projectables) | ||
| if num != 1: | ||
| raise ValueError("Can't have more than one band in a single-band composite") | ||
| mode = attrs.get('mode', 'L') |
There was a problem hiding this comment.
Does mode make sense here? This is for non-image composites technically ("physical quantities").
There was a problem hiding this comment.
Well it could be a single band image in the end...
There was a problem hiding this comment.
True, but that's the default. If someone wants to provide the mode then there is no reason for the default and it should be preserved anyway. Plus this would be adding metadata to a compositor that is "preserving" the metadata. Very image-y.
There was a problem hiding this comment.
or do you mean that mode is already set in attrs ?
There was a problem hiding this comment.
Yes, if the image is an image then it is already set and if not then when this physical quantity gets saved as an image the L mode will be used by default.
flake8 satpy