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.

Sign up for GitHub

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

Jump to bottom

Add VII L2 netCDF-reader. #1219

Merged
merged 11 commits into from Jun 8, 2020
Merged

Add VII L2 netCDF-reader. #1219

merged 11 commits into from Jun 8, 2020

Conversation

sjoro
Copy link
Collaborator

@sjoro sjoro commented May 26, 2020 •
edited by mraspaud

Add EPS-SG VII L2 netCDF reader. Replaces PR #1074 and is based on a most recent master-branch.

  • Tests added
  • Tests passed
  • Passes flake8 satpy
  • Fully documented

mraspaud
mraspaud requested changes May 28, 2020
View reviewed changes
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.

Thanks for the comprehensive PR.

Don't forget to add the reader to index.rst

README.md Outdated Show resolved Hide resolved
satpy/readers/netcdf_utils.py Outdated Show resolved Hide resolved
satpy/readers/netcdf_utils.py Outdated
Comment on lines 149 to 152
full_group_name = base_name + group_name
self.file_content[full_group_name] = group_obj
self._collect_attrs(full_group_name, group_obj)
self.collect_metadata(full_group_name, group_obj)
Copy link
Member

Choose a reason for hiding this comment

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

@gerritholl you worked with this last, would you mind checking this change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good to me. I originally introduced this for performance reasons, I observe no performance difference in the script I used to measure this back then.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for checking it out

satpy/readers/netcdf_utils.py Outdated Show resolved Hide resolved
satpy/readers/netcdf_utils.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_netcdf_utils.py Outdated
@@ -54,7 +54,6 @@ def get_test_content(self, filename, filename_info, filetype_info):
- '/attr/global_attr'
- 'dataset/attr/global_attr'
- 'dataset/shape'
- 'dataset/dimensions'
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you are reverting changes done recently here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same as earlier, netcdf_utilsin thi PR is not from the latest Satpy-master.

satpy/readers/vii_utils.py Outdated Show resolved Hide resolved
satpy/readers/vii_l2_nc.py Show resolved Hide resolved
satpy/readers/vii_base_nc.py Outdated Show resolved Hide resolved
satpy/readers/vii_base_nc.py Outdated Show resolved Hide resolved
@sjoro sjoro marked this pull request as ready for review June 4, 2020 14:30
@sjoro sjoro requested a review from djhoese as a code owner June 4, 2020 14:30
@coveralls
Copy link

coveralls commented Jun 5, 2020 •
edited

Coverage Status

Coverage decreased (-0.002%) to 90.029% when pulling be70840 on sjoro:vii_l2 into 8cfb5df on pytroll:master.

@mraspaud
Copy link
Member

mraspaud commented Jun 8, 2020

Trigger rebuild

@mraspaud mraspaud closed this Jun 8, 2020
@mraspaud mraspaud reopened this Jun 8, 2020
@mraspaud mraspaud added component:readers enhancement code enhancements, features, improvements labels Jun 8, 2020
@codecov
Copy link

codecov bot commented Jun 8, 2020 •
edited

Codecov Report

Merging #1219 into master will decrease coverage by 0.00%.
The diff coverage is 91.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1219      +/-   ##
==========================================
- Coverage   90.03%   90.02%   -0.01%     
==========================================
  Files         216      218       +2     
  Lines       31197    31242      +45     
==========================================
+ Hits        28087    28127      +40     
- Misses       3110     3115       +5
Impacted Files Coverage Δ
satpy/readers/vii_l2_nc.py 83.33% <83.33%> (ø)
satpy/tests/reader_tests/test_vii_l2_nc.py 93.93% <93.93%> (ø)
satpy/scene.py 90.05% <0.00%> (-0.18%) ⬇️

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 8cfb5df...be70840. Read the comment docs.

@mraspaud mraspaud merged commit 0da842a into pytroll:master Jun 8, 2020
@sjoro sjoro deleted the vii_l2 branch June 8, 2020 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mraspaud mraspaud mraspaud requested changes

@gerritholl gerritholl gerritholl left review comments

@djhoese djhoese Awaiting requested review from djhoese

Assignees

@sjoro sjoro

Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sjoro @coveralls @mraspaud @gerritholl