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

Feature Sentinel-3 Level-2 SST #1020

Merged
merged 17 commits into from Jan 27, 2020
Merged

Conversation

eysteinn
Copy link
Contributor

Adds reader for Sentinel-3 Level-2 SST netCDF data.

@coveralls
Copy link

coveralls commented Dec 16, 2019

Coverage Status

Coverage increased (+0.04%) to 87.535% when pulling 773d8fa on eysteinn:feature-slstr_l2 into fa997a0 on pytroll:master.

@codecov
Copy link

codecov bot commented Dec 16, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@b8f35b5). Click here to learn what that means.
The diff coverage is 91.36%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1020   +/-   ##
=========================================
  Coverage          ?   87.53%           
=========================================
  Files             ?      190           
  Lines             ?    29315           
  Branches          ?        0           
=========================================
  Hits              ?    25661           
  Misses            ?     3654           
  Partials          ?        0
Impacted Files Coverage Δ
satpy/readers/generic_image.py 93.33% <ø> (ø)
satpy/readers/hdf4_utils.py 96.49% <ø> (ø)
satpy/readers/nwcsaf_msg2013_hdf5.py 96.15% <ø> (ø)
satpy/readers/abi_l2_nc.py 69.23% <ø> (ø)
satpy/tests/reader_tests/test_seviri_l1b_hrit.py 98.95% <ø> (ø)
satpy/tests/reader_tests/test_ami_l1b.py 94.78% <ø> (ø)
satpy/readers/abi_l1b.py 98.3% <ø> (ø)
satpy/readers/aapp_l1b.py 14.35% <0%> (ø)
satpy/enhancements/mimic.py 0% <0%> (ø)
satpy/tests/test_readers.py 98.02% <100%> (ø)
... and 74 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 b8f35b5...773d8fa. Read the comment docs.

@mraspaud
Copy link
Member

Thanks for the PR @eysteinn ! any particular reason you are using h5py instead of xarray.open_dataset or h5netcdf or netcdf4 ? I'm thinking about the preservation of dimension names and coordinates ?

@eysteinn
Copy link
Contributor Author

@mraspaud Thank you for the comment. There is no specific reason except its usually the first tool I use for this kind of data.
I was not aware of xarray.open_dataset but after looking into it, it seems like a much better choice.
I will look into changing the code to use it instead :)

@eysteinn
Copy link
Contributor Author

@mraspaud forgot to mention that the reader has been updated to use xarray.open_dataset.

@mraspaud
Copy link
Member

thanks for making the switch. Fancy writing a couple of tests next ?

@mraspaud
Copy link
Member

mraspaud commented Dec 28, 2019

Also you'll need to add this reader to the reader table in index.rst

@mraspaud mraspaud added component:readers enhancement code enhancements, features, improvements labels Dec 28, 2019
@eysteinn
Copy link
Contributor Author

@mraspaud Finally got around to writing the tests and modify index.rst

@mraspaud
Copy link
Member

Thanks @eysteinn , reviewing now

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.

Thank you for adding this reader ! I have a couple questions inline, but otherwise my only concern is that the code doesn't seem to have bee run through flake8-docstring. Would you mind installing it and running flake8 on this code again ?

satpy/etc/readers/slstr_l2.yaml Outdated Show resolved Hide resolved
satpy/readers/slstr_l2.py Show resolved Hide resolved
Eysteinn and others added 2 commits January 24, 2020 12:25
Level 2 is better

Co-Authored-By: Martin Raspaud <martin.raspaud@smhi.se>
@eysteinn
Copy link
Contributor Author

Thank you for adding this reader ! I have a couple questions inline, but otherwise my only concern is that the code doesn't seem to have bee run through flake8-docstring. Would you mind installing it and running flake8 on this code again ?

Thanks for the review, I did now know about flake8-docstring. Now that should be fixed.

@mraspaud mraspaud merged commit 17efb75 into pytroll:master Jan 27, 2020
@mraspaud
Copy link
Member

Merged, thanks for the contribution!

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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants