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

Fix bug in OLCI reader that caused multiple error messages to print #945

Merged
merged 8 commits into from Oct 21, 2019
Merged

Fix bug in OLCI reader that caused multiple error messages to print #945

merged 8 commits into from Oct 21, 2019

Conversation

simonrp84
Copy link
Member

@simonrp84 simonrp84 commented Oct 17, 2019

In issue #944 I describe how the OLCI reader prints numerous error messages to the screen during processing. This PR fixed the error messages by switching to the default xarray engine for netCDF4. I have also added functions that explicitly close the netcdf files at the end of the processing. This may or may not be needed (I'm not sure) but I thought I'd copy what the GOES-ABI reader does here.

Lastly, I also split one line of code onto two, which was previously giving a flake8 style error for being too long.

…ted. Also a minor style fix for one line that was too long for flake8
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 85.386% when pulling cf1ab72 on simonrp84:master into 325848a on pytroll:master.

@coveralls
Copy link

coveralls commented Oct 17, 2019

Coverage Status

Coverage increased (+0.4%) to 85.84% when pulling af639bb on simonrp84:master into 325848a on pytroll:master.

@codecov
Copy link

codecov bot commented Oct 17, 2019

Codecov Report

Merging #945 into master will increase coverage by 0.42%.
The diff coverage is 90.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #945      +/-   ##
==========================================
+ Coverage   85.41%   85.84%   +0.42%     
==========================================
  Files         172      172              
  Lines       26048    26222     +174     
==========================================
+ Hits        22248    22509     +261     
+ Misses       3800     3713      -87
Impacted Files Coverage Δ
satpy/tests/reader_tests/test_olci_nc.py 95.6% <100%> (+1.31%) ⬆️
satpy/readers/olci_nc.py 90.9% <76.47%> (+42.57%) ⬆️
satpy/readers/avhrr_l1b_gaclac.py 90.14% <0%> (-1.41%) ⬇️
satpy/tests/test_multiscene.py 97.84% <0%> (ø) ⬆️
satpy/multiscene.py 89.13% <0%> (ø) ⬆️
satpy/readers/modis_l2.py 98.52% <0%> (ø) ⬆️
satpy/readers/seviri_l1b_hrit.py 92.3% <0%> (ø) ⬆️
satpy/tests/enhancement_tests/test_enhancements.py 98.72% <0%> (+1.81%) ⬆️
satpy/enhancements/__init__.py 97.05% <0%> (+7.65%) ⬆️

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 325848a...af639bb. Read the comment docs.

@mraspaud
Copy link
Member

Should we have the default engine to None so as to use xarray's default, or do we want to hardcode it to netcdf4 ?

@simonrp84
Copy link
Member Author

Should we have the default engine to None so as to use xarray's default, or do we want to hardcode it to netcdf4 ?

Good idea, I've changed it to None. I have also added/modified the tests so that coverage is improved a bit.

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.

LGTM, thanks for cleaning up the deprecated stuff :) I just have a few cosmetic items to point out.

satpy/readers/olci_nc.py Show resolved Hide resolved
satpy/readers/olci_nc.py Outdated Show resolved Hide resolved
satpy/readers/olci_nc.py Outdated Show resolved Hide resolved
@mraspaud mraspaud merged commit 1a00661 into pytroll:master Oct 21, 2019
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.

Multiple errors when processing OLCI data.
4 participants