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

Add option to clip negative ABI radiances #2499

Merged
merged 11 commits into from
Jun 26, 2023

Conversation

ghiggi
Copy link
Contributor

@ghiggi ghiggi commented Jun 1, 2023

In this PR, we aim to provide satpy the ability to clip negative radiances (i.e of ABI)

  • Closes #xxxx
  • Tests added
  • Fully documented
  • Add your name to AUTHORS.md if not there already

Related to:

@ghiggi
Copy link
Contributor Author

ghiggi commented Jun 1, 2023

@simonrp84 cc

@coveralls
Copy link

coveralls commented Jun 1, 2023

Pull Request Test Coverage Report for Build 5346533956

  • 66 of 66 (100.0%) changed or added relevant lines in 2 files are covered.
  • 100 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.01%) to 95.425%

Files with Coverage Reduction New Missed Lines %
satpy/composites/init.py 6 91.24%
satpy/readers/avhrr_l1b_gaclac.py 6 95.68%
satpy/readers/olci_nc.py 11 94.2%
satpy/readers/geocat.py 18 88.82%
satpy/readers/seviri_l1b_native.py 21 86.82%
satpy/readers/nwcsaf_nc.py 38 82.0%
Totals Coverage Status
Change from base Build 5141473420: 0.01%
Covered Lines: 47247
Relevant Lines: 49512

💛 - Coveralls

@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Merging #2499 (4d2b9d3) into main (1f97c0b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2499      +/-   ##
==========================================
+ Coverage   94.84%   94.85%   +0.01%     
==========================================
  Files         337      337              
  Lines       49571    49689     +118     
==========================================
+ Hits        47016    47134     +118     
  Misses       2555     2555              
Flag Coverage Δ
behaviourtests 4.41% <0.00%> (-0.01%) ⬇️
unittests 95.47% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
satpy/_config.py 91.57% <ø> (ø)
satpy/readers/abi_l1b.py 98.90% <100.00%> (+0.25%) ⬆️
satpy/tests/reader_tests/test_abi_l1b.py 99.38% <100.00%> (+0.13%) ⬆️

... and 10 files with indirect coverage changes

satpy/readers/abi_base.py Outdated Show resolved Hide resolved
satpy/readers/abi_l1b.py Outdated Show resolved Hide resolved
@ghiggi ghiggi requested a review from djhoese June 2, 2023 10:22
@ghiggi
Copy link
Contributor Author

ghiggi commented Jun 2, 2023

Everythings seems ready for a final review ;)

@djhoese djhoese added enhancement code enhancements, features, improvements component:readers labels Jun 6, 2023
@djhoese djhoese self-assigned this Jun 6, 2023
satpy/readers/abi_l1b.py Outdated Show resolved Hide resolved
satpy/readers/abi_l1b.py Outdated Show resolved Hide resolved
@djhoese
Copy link
Member

djhoese commented Jun 7, 2023

Oh I just realized the configuration documentation should probably be updated too.

doc/source/config.rst Outdated Show resolved Hide resolved
@ghiggi ghiggi requested a review from djhoese June 7, 2023 13:58
doc/source/config.rst Outdated Show resolved Hide resolved
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.

I think I'm good with this. Thanks for getting this done. @ghiggi have you tested it it for your use case and does it work as expected? @simonrp84 any other concerns?

@ghiggi
Copy link
Contributor Author

ghiggi commented Jun 21, 2023

@mraspaud milestone v0.43.0

@mraspaud mraspaud added this to the v0.43.0 milestone Jun 21, 2023
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.

Looks great, just some minor comments inline

satpy/readers/abi_l1b.py Outdated Show resolved Hide resolved
doc/source/config.rst Outdated Show resolved Hide resolved
satpy/readers/abi_l1b.py Show resolved Hide resolved
satpy/tests/reader_tests/test_abi_l1b.py Outdated Show resolved Hide resolved
ghiggi and others added 2 commits June 22, 2023 09:39
Co-authored-by: Martin Raspaud <martin.raspaud@smhi.se>
Co-authored-by: Martin Raspaud <martin.raspaud@smhi.se>
Copy link
Member

@simonrp84 simonrp84 left a comment

Choose a reason for hiding this comment

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

A single suggestion from me, otherwise looks OK.

Out of interest, does GOES-ABI suffer a similar problem at the top end? I know SEVIRI has issues with large radiance out-of-range for the big depth of the sensor (which normally roll back so they look like low radiances) and I'm curious if ABI has the same

doc/source/config.rst Outdated Show resolved Hide resolved
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants