-
Notifications
You must be signed in to change notification settings - Fork 300
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
Change ESSL colorisation approach #3021
Conversation
Change the approach for ESSL colorisation. Remove the dedicated class and instead define a colourmap to be used with the colorize enhancement. Fix the ratio to be the right way around. Fixes and closes 3020.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3021 +/- ##
==========================================
- Coverage 96.11% 96.10% -0.01%
==========================================
Files 383 381 -2
Lines 55604 55544 -60
==========================================
- Hits 53443 53383 -60
Misses 2161 2161
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Pull Request Test Coverage Report for Build 12831451262Details
💛 - Coveralls |
bab99f9
to
982a31c
Compare
Thanks for the PR! Can we have a before/after image comparison? |
Change standard_name back to name in the enhancement, or satpy won't find the correct enhancement and won't use it.
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.
start behave test
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.
start behave test
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.
start behave test
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.
start behave test
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.
start behave test
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.
start behave test
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.
start behave test
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.
start behave test
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.
start behave test
Starting to clone and test the repository pytroll/satpy |
The testing process was executed successfully. See the test results for this pull request here! |
For the image comparison tests, There are also 24 pixels in low level moisture that are different (out of 575280), probably because I calculated the reference image on a different machine and there are rounding differences. |
Something you want to fix in this PR? |
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.
LGTM!
"No code is better than no code"
No. Night microphysics for FCI needs more discussion. And the 24 pixels in low level moisture are below the threshold, rounding differences are allowed. |
Change the approach for ESSL colorisation. Remove the dedicated class and instead define a colourmap to be used with the colorize enhancement. Fix the ratio to be the right way around.
Fixes and closes 3020.
Added an image comparison test for low level moisture over Mali. Because it uses the same instrument (Meteosat-12) as the ones added in #3013, added a column for what scene we are looking at (a simple text label) to the behave tests.