-
Notifications
You must be signed in to change notification settings - Fork 295
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
Fix palette application for pps 2018 products #542
Conversation
Codecov Report
@@ Coverage Diff @@
## master #542 +/- ##
==========================================
+ Coverage 74.94% 75.42% +0.48%
==========================================
Files 136 136
Lines 18657 18774 +117
==========================================
+ Hits 13982 14161 +179
+ Misses 4675 4613 -62
Continue to review full report at Codecov.
|
@@ -162,14 +162,17 @@ def scale_dataset(self, dsid, variable, info): | |||
except AttributeError: | |||
pass | |||
|
|||
if 'palette_meanings' in variable.attrs: | |||
variable.attrs['palette_meanings'] = [int(val) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are NetCDF4 files right? Can't you store a series of numbers in an attribute? I know this is just the reader of these files and not the writer, just curious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's nc files, but I'm not sure attributes can store anything else than atomic types (numbers or strings)
dims=['value', 'band']) | ||
palette.attrs['palette_meanings'] = [2, 3, 4] | ||
cmap, sqpal = cmap_comp.build_colormap(palette, np.uint8, {}) | ||
self.assertTrue(np.allclose(cmap.values, [2, 3, 4])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do np.testing.assert_allclose(...)
instead of self.assertTrue(np.allclose(...))
. You may have known that already and prefer the unittest based assert and if so, no problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that, but I usually use unittest's assert to make use of self
and avoid a style error of not using it.
This PR fixes the palette handling for PPS 2018 products. It allows the use of the
palette_meanings
attribute when available (palette indices).git diff origin/master -- "*py" | flake8 --diff