New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RGBs should never have units, but some do #2066
Comments
I thought I had discussed this before, but couldn't find a corresponding issue, so maybe it was only in slack. I apologise if this is a duplicate for an issue I couldn't find right now. |
So I'm a little torn on this, but only barely. The units and calibration metadata are retained because the input bands all have the same value so the The calibration is an interesting problem though. I think technically you can request a composite and specify a different calibration and Satpy will allow that to continue on through processing as if it was the "higher" level of calibration. I think @simonrp84 tried to do something like that before, but I'm not sure how that went. So the question is, is it wrong for that RGB to have a I guess the "safest" answer is to say neither should be present (or maybe |
I don't feel strongly about |
I have left a comment on #1985 with some thoughts on why non-quantities should not have units associated with them. Those thoughts apply to RGBs as well. |
Just to clarify, the main failure is that it is trying to do this with a 3D image (RGB/A) and not a 2D image, right?
This is a good point. As it is right now any resulting metadata in a composite can be thought of as existing because all inputs had that piece of metadata with that exact value. But that doesn't mean it is accurate to the RGB and it causes complications when input metadata was not all the same. So either the metadata handling should be updated to produce multiple inputs or it should be removed. Due to the complications of the multiple values for low-level parameters like One thought, what if a user wanted to force the |
Describe the bug
When loading the snow_age RGB with the viirs_sdr reader, the resulting RGB has the attribute
calibration
set toreflectance
and the attributeunits
set to%
. This is incorrect.It also leads to a failure in the ninjotiff writer, which unsuccessfully attempts a unit conversion, resulting in a NotImplementedError.
To Reproduce
This code checks the value for calibration and units for all loadable composites:
Expected behavior
I expect that an RGB should never have calibration or units attributes set.
The DNB composites are a different issue; see #1985.
Actual results
The following composites unexpectedly have calibration and units attributes defined:
The DNB composites have those set as well, but that is more debatable, as the output is still mode L, so one could argue units are still valid. See #1985 for a separate issue on this.
The composite
ssec_fog
also has units set, but this is a single-channel output so could be correct (the values I get range from -80 to +15, with the metadata claiming calibration in brightness temperature with units of K, so this is also wrong, but in a different way than for the RGBs).There may be other RGBs that unexpectedly have units; this lists just the ones loadable with VIIRS.
I cannot include the full output here (GitHub error: comment is too long), therefore an excerpt only.
output of MCVE
Environment Info:
The text was updated successfully, but these errors were encountered: