satpy icon indicating copy to clipboard operation
satpy copied to clipboard

Add a reader for EPS-SG Ice Cloud Imager

Open BengtRydberg opened this issue 2 years ago • 9 comments

Adding a level1b reader for EPS-SG Ice Cloud Imager (ICI) that will be one the instrument on metop-SGB and will be launched around 2025. Such a reader is presently not available here, and the aim is to use satpy within ICI level2 processing chain.

The reader is configured to be able to read some of the datasets within the file, but not all.

The ICI Level 1B Product Format Specification V3A: https://www.eumetsat.int/media/47582 Eumetsat has published some test data here: https://www.eumetsat.int/release-simulated-eps-sg-mwi-ici-test-data-2021

  • [ ] Closes #xxxx (there is no related issue, should I add one?)
  • [x] Tests added
  • [ ] Fully documented (not 100% sure what is expected)
  • [x] Add your name to AUTHORS.md if not there already

BengtRydberg avatar May 31 '22 14:05 BengtRydberg

Codecov Report

Merging #2118 (4bb68cd) into main (aa7f0dd) will increase coverage by 0.03%. The diff coverage is 97.55%.

@@            Coverage Diff             @@
##             main    #2118      +/-   ##
==========================================
+ Coverage   93.99%   94.03%   +0.03%     
==========================================
  Files         287      289       +2     
  Lines       44092    44541     +449     
==========================================
+ Hits        41446    41884     +438     
- Misses       2646     2657      +11     
Flag Coverage Δ
behaviourtests 4.73% <0.00%> (-0.05%) :arrow_down:
unittests 94.69% <97.55%> (+0.02%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
satpy/readers/ici_l1b_nc.py 94.88% <94.88%> (ø)
satpy/tests/reader_tests/test_ici_l1b_nc.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jun 01 '22 13:06 codecov[bot]

Coverage Status

Coverage increased (+0.03%) to 94.511% when pulling 6e7c046047c23635e3b134e259887824286b49fc on BengtRydberg:ici_reader into 2779675fd72826f8df166c46a1b0d59767ba720f on pytroll:main.

coveralls avatar Jun 01 '22 13:06 coveralls

@BengtRydberg Nice with the latest changes! I think it would be nice if you also added the appropriate identification keys in the yaml reader definitions, see for example what @adybbroe is doing here: https://github.com/pytroll/satpy/pull/2120

mraspaud avatar Jun 08 '22 11:06 mraspaud

@BengtRydberg Nice with the latest changes! I think it would be nice if you also added the appropriate identification keys in the yaml reader definitions, see for example what @adybbroe is doing here: #2120

Agree. Added the identification keys.

BengtRydberg avatar Jun 08 '22 12:06 BengtRydberg

LGTM. Very nice work! Just one thing: Can you add the reader to the list of supported readers in the documentation? See the index.rst file

Also a question, does the order of the coordinates matter? I have the lons first and the lats second, I see you have it the other way around. Have you tested reading, resampling and displaying one of the channels?

I have added the readers to the index file. As I understand the order of the coordinates does not matter, since in the "coordinates section" of the yaml file one specify the standard name of the coordinate, such that a dataset is created with a proper area definition with lats and lons that I guess is used for resampling. I have plotted the data and it looks like I expect.

BengtRydberg avatar Jun 28 '22 12:06 BengtRydberg

You aren't wrong about the coordinates being sorted by standard name, but I'm also not sure I like the inconsistency between these readers and others.

djhoese avatar Jun 28 '22 15:06 djhoese

You aren't wrong about the coordinates being sorted by standard name, but I'm also not sure I like the inconsistency between these readers and others.

Agree. I have changed the order. I was just not sure what was the "standard order" but I can see that this is how it is done in the yaml file for most of the readers in the repo.

BengtRydberg avatar Jun 29 '22 07:06 BengtRydberg

Could you merge main and resolve the merge conflicts? I think merging main will also take care of the SEVIRI test failures.

gerritholl avatar Jul 29 '22 09:07 gerritholl

Could you merge main and resolve the merge conflicts? I think merging main will also take care of the SEVIRI test failures.

Yes, I have merged main.

BengtRydberg avatar Aug 08 '22 10:08 BengtRydberg