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

Update AVHRR EPS reader to read cloud flags information #2639

Merged
merged 5 commits into from
Nov 17, 2023

Conversation

fwfichtner
Copy link
Contributor

@fwfichtner
Copy link
Contributor Author

We can take care of the CI problems if this is something you'd be interested to merge.

@djhoese
Copy link
Member

djhoese commented Nov 14, 2023

You mentioned in your issue that you're using an old version of Satpy. Does that mean that this functionality and variable used to work, but was removed?

Copy link

codecov bot commented Nov 14, 2023

Codecov Report

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

Comparison is base (6fb1d1e) 95.20% compared to head (4181504) 95.22%.
Report is 45 commits behind head on main.

Files Patch % Lines
satpy/readers/eps_l1b.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2639      +/-   ##
==========================================
+ Coverage   95.20%   95.22%   +0.02%     
==========================================
  Files         354      356       +2     
  Lines       51370    51547     +177     
==========================================
+ Hits        48908    49088     +180     
+ Misses       2462     2459       -3     
Flag Coverage Δ
behaviourtests 4.22% <0.00%> (-0.02%) ⬇️
unittests 95.85% <96.29%> (+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.

@coveralls
Copy link

coveralls commented Nov 14, 2023

Pull Request Test Coverage Report for Build 6906945466

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 26 of 27 (96.3%) changed or added relevant lines in 2 files are covered.
  • 12 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.02%) to 95.803%

Changes Missing Coverage Covered Lines Changed/Added Lines %
satpy/readers/eps_l1b.py 19 20 95.0%
Files with Coverage Reduction New Missed Lines %
satpy/tests/modifier_tests/test_angles.py 1 99.5%
satpy/modifiers/angles.py 2 99.22%
satpy/readers/nwcsaf_nc.py 4 82.21%
satpy/tests/reader_tests/modis_tests/_modis_fixtures.py 5 98.23%
Totals Coverage Status
Change from base Build 6847297703: 0.02%
Covered Lines: 49214
Relevant Lines: 51370

💛 - Coveralls

@fwfichtner
Copy link
Contributor Author

Does that mean that this functionality and variable used to work, but was removed?

No, AFAIK this functionality was never there. We just forked, added this and then forgot about it.

@djhoese djhoese added enhancement code enhancements, features, improvements component:readers labels Nov 14, 2023
@djhoese
Copy link
Member

djhoese commented Nov 14, 2023

Ah ok, great. We'd really like a unit test to test this new variable. Thanks.

@djhoese
Copy link
Member

djhoese commented Nov 17, 2023

pre-commit.ci autofix

@djhoese
Copy link
Member

djhoese commented Nov 17, 2023

Thanks @fwfichtner. I did some refactoring of the code to make some of our checkers happy. It wasn't that your code was bad in anyway, just that your code being added tipped the scale for our checkers and revealed that the code was too complex. It is a little "cleaner" now.

I'll merge this when CI tests pass.

@djhoese djhoese self-assigned this Nov 17, 2023
@djhoese djhoese merged commit ace3999 into pytroll:main Nov 17, 2023
19 checks passed
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.

Update AVHRR EPS reader to read cloud flags information
3 participants