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

Add reader for FIDUCEO MVIRI FCDR data #1427

Merged
merged 47 commits into from Dec 8, 2020

Conversation

sfinkens
Copy link
Member

@sfinkens sfinkens commented Nov 6, 2020

Add a reader for FIDUCEO MVIRI FCDR data in netCDF format. Both full and easy FCDR are supported.


Example usage:

from satpy import Scene

scn = Scene(filenames=['FIDUCEO_FCDR_L15_MVIRI_MET7-57.0...'],
            reader='mviri_l1b_fiduceo_nc',
            reader_kwargs={'mask_bad_quality': False})
scn.load(['VIS', 'WV', 'IR'], upper_right_corner='NE')

Example images (with upper_right_corner='native'):

fiduceo_mviri_fcdr_VIS
fiduceo_mviri_fcdr_WV
fiduceo_mviri_fcdr_IR


Difference between lon/lat from area definition and "static FCDR"

lon_diff_VIS_57_deg
lat_diff_VIS_57_deg

  • Tests added
  • Passes flake8 satpy
  • Fully documented

@ghost
Copy link

ghost commented Nov 6, 2020

Congratulations 🎉. DeepCode analyzed your code in 3.078 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@codecov
Copy link

codecov bot commented Nov 6, 2020

Codecov Report

Merging #1427 (c5b5331) into master (2d5bfdb) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1427      +/-   ##
==========================================
+ Coverage   90.58%   90.63%   +0.05%     
==========================================
  Files         236      240       +4     
  Lines       33797    34582     +785     
==========================================
+ Hits        30615    31345     +730     
- Misses       3182     3237      +55     
Flag Coverage Δ
behaviourtests 4.47% <0.00%> (?)
unittests 91.11% <100.00%> (?)

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

Impacted Files Coverage Δ
satpy/readers/_geos_area.py 100.00% <100.00%> (ø)
satpy/readers/mviri_l1b_fiduceo_nc.py 100.00% <100.00%> (ø)
satpy/tests/reader_tests/test_geos_area.py 100.00% <100.00%> (ø)
...py/tests/reader_tests/test_mviri_l1b_fiduceo_nc.py 100.00% <100.00%> (ø)
satpy/utils.py 24.09% <0.00%> (-46.39%) ⬇️
satpy/readers/nwcsaf_msg2013_hdf5.py 94.87% <0.00%> (-1.29%) ⬇️
satpy/resample.py 89.69% <0.00%> (-1.09%) ⬇️
satpy/readers/hdf5_utils.py 91.42% <0.00%> (-1.00%) ⬇️
satpy/readers/viirs_sdr.py 85.63% <0.00%> (-0.21%) ⬇️
satpy/tests/test_yaml_reader.py 99.83% <0.00%> (ø)
... and 26 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 2d5bfdb...c5b5331. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 90.237% when pulling 0b6e49d on sfinkens:feature-fiduceo-mviri into 979608a on pytroll:master.

@simonrp84
Copy link
Member

Do the example images correspond to the example code?
I ask because you have upper_right_corner='NE' in the code but this is clearly not the case for the images!
Just want to check in case there's an issue there, unfortunately I don't have any data to test with :)

Copy link
Member

@simonrp84 simonrp84 left a comment

Choose a reason for hiding this comment

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

LGTM, can't see anything wrong or in need of improvement in there!

@sfinkens
Copy link
Member Author

sfinkens commented Nov 9, 2020

Do the example images correspond to the example code?

@simonrp84 You're right, this is a bit confusing. The sample images were created with upper_right_corner='native' .

@mraspaud mraspaud added component:readers enhancement code enhancements, features, improvements labels Nov 16, 2020
Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

Looks good! I some comments, but please write the tests before addressing them 👍

Also, don't forget to add the reader to the table in the index.rst file.

satpy/readers/mviri_l1b_fiduceo_nc.py Outdated Show resolved Hide resolved
satpy/readers/mviri_l1b_fiduceo_nc.py Outdated Show resolved Hide resolved
Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

I still have a couple of comments. Also, how is it with sun-earth distance in these data?

satpy/readers/_geos_area.py Outdated Show resolved Hide resolved
satpy/readers/mviri_l1b_fiduceo_nc.py Outdated Show resolved Hide resolved
@mraspaud
Copy link
Member

mraspaud commented Nov 23, 2020

Also could you try to address the codebeat and deepcode issues?

@sfinkens
Copy link
Member Author

sfinkens commented Nov 24, 2020

I'll try :)

Also could you try to address the codebeat and deepcode issues?

@mraspaud
Copy link
Member

Do. Or do not. There is no try.

;)

@sfinkens
Copy link
Member Author

Retry DeepCode

@mraspaud
Copy link
Member

@sfinkens looks really good now! Almost there, just three lines that miss coverage!

@sfinkens sfinkens added this to Needs Review in PCW Autumn 2020 Nov 30, 2020
@mraspaud mraspaud merged commit 0cfe84d into pytroll:master Dec 8, 2020
PCW Autumn 2020 automation moved this from Needs Review to Done Dec 8, 2020
@sfinkens sfinkens deleted the feature-fiduceo-mviri branch December 8, 2020 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants