Skip to content
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

Apply valid_range in MiRS reader when present #1655

Merged
merged 21 commits into from
May 3, 2021

Conversation

joleenf
Copy link
Contributor

@joleenf joleenf commented Apr 30, 2021

This fixes a bug in which the valid range was not used. Fix checks for a valid range in the attributes and applies the range if present. There is a new check in the test code which determines if valid_range is removed from the attributes after loading the data.

Remove platform name from parameterize in last test for kwargs because
they are not used in that test.  Add a test for the NOAA-20 platform
when the limb_correction assertions are tested and platform name is
tested.
Merge remote-tracking branch 'upstream/master'
Fixes a bug in which valid range was ignored.
@codecov
Copy link

codecov bot commented Apr 30, 2021

Codecov Report

Merging #1655 (879fce4) into master (cefc56b) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1655      +/-   ##
==========================================
+ Coverage   92.61%   92.65%   +0.03%     
==========================================
  Files         258      258              
  Lines       37783    37948     +165     
==========================================
+ Hits        34994    35161     +167     
+ Misses       2789     2787       -2     
Flag Coverage Δ
behaviourtests 4.83% <0.00%> (+0.01%) ⬆️
unittests 92.79% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
satpy/readers/mirs.py 73.38% <100.00%> (+0.83%) ⬆️
satpy/tests/reader_tests/test_mirs.py 100.00% <100.00%> (ø)
satpy/_compat.py 81.81% <0.00%> (-18.19%) ⬇️
satpy/tests/test_dataset.py 100.00% <0.00%> (ø)
satpy/readers/electrol_hrit.py 91.66% <0.00%> (ø)
satpy/tests/reader_tests/test_aapp_l1b.py 100.00% <0.00%> (ø)
satpy/tests/enhancement_tests/test_enhancements.py 100.00% <0.00%> (ø)
satpy/tests/test_scene.py 99.44% <0.00%> (+<0.01%) ⬆️
satpy/scene.py 92.29% <0.00%> (+0.11%) ⬆️
satpy/enhancements/__init__.py 97.50% <0.00%> (+0.14%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cefc56b...879fce4. Read the comment docs.

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a couple questions. Also, how hard would it be to check the data in the tests to make sure the valid range was applied?

@@ -356,6 +356,17 @@ def _fill_data(self, data_arr, attrs):
data_arr = data_arr.where(data_arr != fill_value, fill_out)
return data_arr, attrs

def _get_valid_range(self, data_arr, attrs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def _get_valid_range(self, data_arr, attrs):
def _apply_valid_range(self, data_arr, attrs):

Really nitpicking here. Not necessary unless you make other changes.

joleenf added 2 commits April 30, 2021 13:17
Change get_valid_range=>apply_valid_range and check for case of valid_range not None
only read fake_data one time, not every time check_valid_range is called.
@joleenf joleenf requested a review from djhoese May 3, 2021 11:13
@djhoese djhoese changed the title Apply valid_range when in attributes Apply valid_range in MiRS reader when present May 3, 2021
Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@djhoese djhoese merged commit 6df2f10 into pytroll:master May 3, 2021
@joleenf joleenf deleted the mirs_validrange branch May 15, 2021 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants