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 Gcom-C sgli reader #1094

Merged
merged 22 commits into from
Dec 4, 2023
Merged

Add Gcom-C sgli reader #1094

merged 22 commits into from
Dec 4, 2023

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Feb 28, 2020 •
edited
Loading

Adds a reader for the SGLI data.

TODO:

  • Remove the no-data stripes on the sides
  • Add all channels for the instrument in the yaml file
  • Add support for polarized channels and IR channels
  • Take care of the saturation
  • Daskify the interpolation of angles and lon/lat (requires Fix geogrid chunking to accept "auto" and to preserve dtype python-geotiepoints#61 to be merged and released)
  • Make sure poles and dateline areas are interpolated correctly (delegated to geotiepoints)
  • Check that units and standard names are correct

And the usual:

@mraspaud mraspaud added enhancement code enhancements, features, improvements help wanted component:readers labels Feb 28, 2020
@mraspaud mraspaud self-assigned this Feb 28, 2020
@mraspaud mraspaud changed the title Add first draft of gcom-c sgli reader Add Gcom-C sgli reader Feb 28, 2020
@mraspaud
Copy link
Member Author

For some reason the rayleigh scattering correction doesn't work yet.
First result with true_color_raw:
sgli_raw

@codecov
Copy link

codecov bot commented Feb 28, 2020 •
edited
Loading

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (e338294) 95.29% compared to head (728631e) 95.32%.
Report is 7 commits behind head on main.

Files Patch % Lines
satpy/readers/sgli_l1b.py 95.86% 6 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1094      +/-   ##
==========================================
+ Coverage   95.29%   95.32%   +0.02%     
==========================================
  Files         369      371       +2     
  Lines       52040    52435     +395     
==========================================
+ Hits        49591    49982     +391     
- Misses       2449     2453       +4
Flag Coverage Δ
behaviourtests 4.16% <0.00%> (-0.04%) ⬇️
unittests 95.93% <98.44%> (+0.02%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mraspaud
Copy link
Member Author

Rayleigh correction fixed.

@coveralls
Copy link

coveralls commented Feb 28, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.1%) to 89.194% when pulling 5a0b66c on mraspaud:feature-sgli-reader into f59da8b on pytroll:master.

@mraspaud
Copy link
Member Author

mraspaud commented Mar 4, 2020 •
edited
Loading

True color

sgli_tc

@CocoasColas CocoasColas mentioned this pull request Oct 25, 2023
Closed
@mraspaud mraspaud marked this pull request as ready for review November 29, 2023 07:40
djhoese
djhoese reviewed Nov 30, 2023
View reviewed changes
satpy/readers/sgli_l1b.py Outdated
Comment on lines 92 to 101
if key["name"].startswith(("VN", "SW", "P")):
dataset = self.get_visible_dataset(key, dataset)
elif key["name"].startswith("TI"):
dataset = self.get_ir_dataset(key, dataset)
elif key["name"].startswith(("longitude", "latitude")):
dataset = self.get_lon_lats(key)
elif "angle" in key["name"]:
dataset = self.get_angles(key)
else:
raise NotImplementedError()
Copy link
Member

Choose a reason for hiding this comment

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

I don't feel very strongly about this, but since code scene is complaining about it, is there a cleaner way to do this? Maybe with a dictionary mapping to functions? That might allow the NotImplementedError to actually be a KeyError which is maybe more clear to the user/log? Either way, maybe an error message should be put in the NotImplementedError?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to avoid having a dictionary as the list of channels and other dataset will be quite large. I'll just extract the method for now and see what code scene says about it. I'm also changing the NotImplementedError into a KeyError with a message.

djhoese
djhoese requested changes Nov 30, 2023
View reviewed changes
satpy/tests/reader_tests/test_sgli_l1b.py Outdated Show resolved Hide resolved
satpy/readers/sgli_l1b.py Outdated Show resolved Hide resolved
@djhoese
Copy link
Member

djhoese commented Dec 4, 2023

I've restarted the RTD build. Looks like it got hung up downloading some intersphinx objects.inv files.

@djhoese
Copy link
Member

djhoese commented Dec 4, 2023

Ah the pooch documentation site is down. Have to wait until it comes back.

@mraspaud
Copy link
Member Author

mraspaud commented Dec 4, 2023

Looks good now, merging

@mraspaud mraspaud merged commit 661e2d3 into pytroll:main Dec 4, 2023
17 of 19 checks passed
@mraspaud mraspaud deleted the feature-sgli-reader branch December 11, 2023 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djhoese djhoese djhoese approved these changes

Assignees

@mraspaud mraspaud

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

Successfully merging this pull request may close these issues.

GCOM-C Support (Continued) Add a reader for GCOM-C Level 1 data
3 participants
@mraspaud @coveralls @djhoese