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

Bugfixes in the Electro-L reader #726

Merged
merged 5 commits into from
Apr 24, 2019
Merged

Bugfixes in the Electro-L reader #726

merged 5 commits into from
Apr 24, 2019

Conversation

simonrp84
Copy link
Member

@simonrp84 simonrp84 commented Apr 16, 2019

Ernst made a bug report in the google group: https://groups.google.com/forum/#!topic/pytroll/Dvvp9z2bIdc
Stating that he could not process or save images from the Electro-L satellite. This was for two reasons:

  1. In the satpy/etc/readers/electrol_hrit.yaml the sensor was incorrectly named electrol rather than msu-gs.
  2. A call to map_blocks in the calibration routine was not passing the look-up table, meaning that a TypeError: _getitem() missing 1 required positional argument: 'lut' was being raised.

This PR fixes both of these problems by adding the correct sensor name and by passing lut to the `_getitem" call.

…ly there was a bug that caused an error due to a map_blocks call not passing the correct variable.
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.

Looks good to me and I see why the map_blocks was failing. My fault, so thanks for fixing it.

I think in the long term we should consider renaming the reader since electrol is technically the satellite and not the sensor (even though OSCAR lists the sensor as "Electrol Imager").

@codecov
Copy link

codecov bot commented Apr 16, 2019

Codecov Report

Merging #726 into master will increase coverage by 0.42%.
The diff coverage is 96.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #726      +/-   ##
==========================================
+ Coverage   80.02%   80.44%   +0.42%     
==========================================
  Files         147      148       +1     
  Lines       21498    21612     +114     
==========================================
+ Hits        17203    17385     +182     
+ Misses       4295     4227      -68
Impacted Files Coverage Δ
satpy/readers/electrol_hrit.py 92% <100%> (+57.6%) ⬆️
satpy/tests/reader_tests/__init__.py 97.77% <100%> (+0.05%) ⬆️
satpy/tests/reader_tests/test_electrol_hrit.py 96.46% <96.46%> (ø)

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 1849b17...085e472. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 80.017% when pulling ba2f3f2 on simonrp84:master into 1849b17 on pytroll:master.

@coveralls
Copy link

coveralls commented Apr 16, 2019

Coverage Status

Coverage increased (+0.4%) to 80.423% when pulling 085e472 on simonrp84:master into 1849b17 on pytroll:master.

@mraspaud
Copy link
Member

Lgtm, feel free to merge!

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

@mraspaud mraspaud merged commit b925fae into pytroll:master Apr 24, 2019
@mraspaud mraspaud added this to the v0.15 milestone Apr 24, 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.

4 participants