Add a reader for NOAA GOES-R ABI L2+ products (abi_l2_nc) #767

yufeizhu600 · 2019-05-16T18:12:28Z

These products have been documented in GOES-R PUG file : https://www.goes-r.gov/products/docs/PUG-L2+-vol5.pdf.

Tests added
Tests passed
Passes git diff origin/master -- "*py" | flake8 --diff
Fully documented

codecov · 2019-05-16T18:27:57Z

Codecov Report

Merging #767 into master will increase coverage by 0.09%.
The diff coverage is 85.37%.

@@            Coverage Diff             @@
##           master     #767      +/-   ##
==========================================
+ Coverage   83.98%   84.07%   +0.09%     
==========================================
  Files         167      170       +3     
  Lines       24587    24673      +86     
==========================================
+ Hits        20649    20744      +95     
+ Misses       3938     3929       -9

Impacted Files	Coverage Δ
satpy/readers/abi_l1b.py	`98.3% <100%> (+3.81%)`	⬆️
satpy/tests/reader_tests/test_scmi.py	`96.19% <100%> (ø)`	⬆️
satpy/tests/reader_tests/__init__.py	`98.11% <100%> (+0.03%)`	⬆️
satpy/readers/abi_l2_nc.py	`33.33% <33.33%> (ø)`
satpy/tests/reader_tests/test_abi_l1b.py	`96.36% <83.33%> (-0.86%)`	⬇️
satpy/readers/abi_base.py	`90.56% <90.56%> (ø)`
satpy/tests/reader_tests/test_abi_l2_nc.py	`94.02% <94.02%> (ø)`
satpy/tests/test_resample.py	`85.6% <0%> (-0.3%)`	⬇️
satpy/tests/writer_tests/test_cf.py	`98.53% <0%> (-0.03%)`	⬇️
... and 10 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 44239a0...f07a8d6. Read the comment docs.

coveralls · 2019-05-16T18:29:11Z

Coverage increased (+0.09%) to 84.076% when pulling f07a8d6 on yufeizhu600:master into 44239a0 on pytroll:master.

mraspaud

Thanks you for the the very nice PR!
A few comments

The reader you provide it very similar to the abi_l1b reader, I suggest they get factorized
You need to add the tests to the suite in satpy/tests/reader_tests/init.py
Don't forget to add you name to the AUTHORS.md!

satpy/readers/abi_l2.py

mraspaud · 2019-05-16T21:36:07Z

Also you should add this reader to the list of readers in the documentation (doc/src/index.rst)

Conflicts: AUTHORS.md satpy/tests/reader_tests/__init__.py

djhoese · 2019-06-26T20:56:15Z

Looks like you'll need to update the mocking in the scmi writer tests to point to the new abi module.

AUTHORS.md

djhoese

A couple small things I'd still like changed. It looks like some changes from master got lost in your most recent merge.

doc/source/index.rst

djhoese · 2019-07-02T16:08:30Z

satpy/readers/abi_l1b.py

@@ -158,47 +92,6 @@ def get_dataset(self, key, info):



Github won't let me comment any higher, but just below line 80 there should be a line remove _Unsigned which was done in #838.

djhoese · 2019-07-02T16:08:58Z

satpy/readers/abi_l2_nc.py

+        variable.attrs.pop('_FillValue', None)
+        variable.attrs.pop('scale_factor', None)
+        variable.attrs.pop('add_offset', None)
+        variable.attrs.pop('valid_range', None)


You probably want to pop _Unsigned here too.

satpy/tests/reader_tests/test_abi_l2_nc.py

satpy/readers/abi_base.py

Conflicts: AUTHORS.md

djhoese · 2019-07-03T13:34:18Z

It looks like your tests are failing because you were depending on .keys() since you defined def keys(self) in your test handlers. You'll probably need to create a fake __contains__ method for the self.nc attribute in your fake handler.

djhoese

Sorry I keep coming up with things for this, but I found one more thing with the lon/lat units (see comment below).

Other than that, I was going to ask again where the dataset names come from but I see they are the names in the files. I officially would like to say they are ugly, but I like the consistency of matching the file variables more so I guess leave them as-is.

satpy/etc/readers/abi_l2_nc.yaml

djhoese

For the record, I approve this PR but have moved the resolved conflicts to another PR #904

Yufei Zhu added 2 commits May 15, 2019 17:20

Add GOES-R ABI l2+ products reader.

9ab3d70

add unit test for GOESR-R ABI l2+ reader

7954674

yufeizhu600 requested review from djhoese and mraspaud as code owners May 16, 2019 18:12

remove trailing spaces in abi_l2.yaml

ac19f1f

yufeizhu600 changed the title ~~This PR add a reader for NOAA GOES-R ABI L2+ products in netCDF format. These products have been documented in GOES-R PUG file https://www.goes-r.gov/products/docs/PUG-L2+-vol5.pdf.~~ This PR add a reader for NOAA GOES-R ABI L2+ products in netCDF format. May 16, 2019

pnuu added component:readers enhancement code enhancements, features, improvements PCW Pytroll Contributors' Week labels May 16, 2019

fix codefactor format issues

e0d1ff2

djhoese changed the title ~~This PR add a reader for NOAA GOES-R ABI L2+ products in netCDF format.~~ Add a reader for NOAA GOES-R ABI L2+ products (nc_abi_l2_fdc) May 16, 2019

djhoese changed the title ~~Add a reader for NOAA GOES-R ABI L2+ products (nc_abi_l2_fdc)~~ Add a reader for NOAA GOES-R ABI L2+ products (abi_l2) May 16, 2019

fix trailing space issue one more time.

e875a9e

mraspaud requested changes May 16, 2019

View reviewed changes

satpy/readers/abi_l2.py Outdated Show resolved Hide resolved

satpy/readers/abi_l2.py Outdated Show resolved Hide resolved

satpy/readers/abi_l2.py Outdated Show resolved Hide resolved

jhbravo mentioned this pull request May 29, 2019

ABI l2 reader #798

Closed

6 tasks

Yufei Zhu and others added 10 commits June 6, 2019 19:06

Merge branch 'master' of https://github.com/pytroll/satpy

3260e46

refactored the ABI l2 reader along with ABI L1b reader.

653a4b7

add nc_abi_l2 reader tests suits into tests/_init__ file

424948a

Modified AUTHORS.md

6777b32

Modified nc_abi_l2.yaml file

5197a39

fix 'file_key' retrieve bug.

07c9ee1

rename abi_l2 to nc_abi_l2 reader in the tests

41455f9

remove reader abi_l2.py and tests

d6c6ad7

fixed CMIP reading bug in the nc_abi_l2.yaml file

15747a0

removed some unused imports, removed tailing whitespaces

109b840

yufeizhu600 and others added 5 commits June 19, 2019 14:55

Merge branch 'master' of https://github.com/yufeizhu600/satpy

1ac6c37

Conflicts: AUTHORS.md satpy/tests/reader_tests/__init__.py

solve merge conflicts

dc10d10

remove unused import in abi l1b test

9e02aa9

Merge branch 'master' into master

269dcc3

remove blank line contains whtiespace

180f558

yufeizhu600 added 2 commits July 2, 2019 15:25

update test_scmi to mock abi_base

5567fce

Merge branch 'nc_abi_l2_reader'

477d14b

djhoese reviewed Jul 2, 2019

View reviewed changes

AUTHORS.md Outdated Show resolved Hide resolved

djhoese requested changes Jul 2, 2019

View reviewed changes

yufeizhu600 added 5 commits July 2, 2019 16:33

Update AUTHORS.md

16ec228

Pop '_Unsigned' from attrs

76c768c

use self.nc instead of self.nc.keys() to check variable for efficiency

1f89d85

correct copyright year in abi_l2 reader/test

a3f13dc

Merge branch 'nc_abi_l2_reader'

a807a68

Conflicts: AUTHORS.md

yufeizhu600 added 3 commits July 4, 2019 18:58

remove not needed function keys() from unittest

15c3b96

add __contains__ to test FakeDataset class

9d35e77

Merge branch 'nc_abi_l2_reader'

92f3645

djhoese requested changes Jul 6, 2019

View reviewed changes

satpy/etc/readers/abi_l2_nc.yaml Outdated Show resolved Hide resolved

yufeizhu600 added 2 commits July 10, 2019 14:23

update the units of lat/lon in abi_l2 yaml file

ad2e864

remove tailing spaces in abi_l2 yaml file

fb16a1b

djhoese reviewed Jul 18, 2019

View reviewed changes

satpy/etc/readers/abi_l2_nc.yaml Outdated Show resolved Hide resolved

correct a typo in abi_l2_nc.yaml file.

f07a8d6

djhoese mentioned this pull request Aug 9, 2019

Fix ABI L1B coordinates to be equivalent at all resolutions #872

Merged

2 tasks

djhoese mentioned this pull request Sep 17, 2019

Add a reader for NOAA GOES-R ABI L2+ products (abi_l2_nc) #904

Merged

djhoese approved these changes Sep 17, 2019

View reviewed changes

djhoese merged commit f07a8d6 into pytroll:master Sep 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a reader for NOAA GOES-R ABI L2+ products (abi_l2_nc) #767

Add a reader for NOAA GOES-R ABI L2+ products (abi_l2_nc) #767

yufeizhu600 commented May 16, 2019 •

edited by mraspaud

Loading

codecov bot commented May 16, 2019 •

edited

Loading

coveralls commented May 16, 2019 •

edited

Loading

mraspaud left a comment

mraspaud commented May 16, 2019

djhoese commented Jun 26, 2019

djhoese left a comment

djhoese Jul 2, 2019

djhoese Jul 2, 2019

djhoese commented Jul 3, 2019

djhoese left a comment

djhoese left a comment

Add a reader for NOAA GOES-R ABI L2+ products (abi_l2_nc) #767

Add a reader for NOAA GOES-R ABI L2+ products (abi_l2_nc) #767

Conversation

yufeizhu600 commented May 16, 2019 • edited by mraspaud Loading

codecov bot commented May 16, 2019 • edited Loading

Codecov Report

coveralls commented May 16, 2019 • edited Loading

mraspaud left a comment

Choose a reason for hiding this comment

mraspaud commented May 16, 2019

djhoese commented Jun 26, 2019

djhoese left a comment

Choose a reason for hiding this comment

djhoese Jul 2, 2019

Choose a reason for hiding this comment

djhoese Jul 2, 2019

Choose a reason for hiding this comment

djhoese commented Jul 3, 2019

djhoese left a comment

Choose a reason for hiding this comment

djhoese left a comment

Choose a reason for hiding this comment

yufeizhu600 commented May 16, 2019 •

edited by mraspaud

Loading

codecov bot commented May 16, 2019 •

edited

Loading

coveralls commented May 16, 2019 •

edited

Loading