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

Fix dataset attribute typo and reduce amount of categorical dataset filtering in fci_l2_nc reader #2049

Merged
merged 4 commits into from
Mar 10, 2022

Conversation

strandgren
Copy link
Collaborator

This PR fixes two minor bugs introduced with the recent merge of PR1927.

  1. Due to a typo in some of the L2PF filenames for the projection attribute inverse_flattening, the reader did not work properly. A temporary workaround has been implemented for the time being until the L2PF test files are correctly formatted.
  2. The filtering of some categorical dataset flags has been removed as the information can be useful.
  • Test modified/corrected accordingly

@codecov
Copy link

codecov bot commented Mar 9, 2022

Codecov Report

Merging #2049 (dc5e03e) into main (c66368c) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2049      +/-   ##
==========================================
+ Coverage   93.73%   93.77%   +0.03%     
==========================================
  Files         282      282              
  Lines       42080    42092      +12     
==========================================
+ Hits        39444    39472      +28     
+ Misses       2636     2620      -16     
Flag Coverage Δ
behaviourtests 4.72% <0.00%> (-0.01%) ⬇️
unittests 94.31% <100.00%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
satpy/readers/fci_l2_nc.py 99.45% <100.00%> (ø)
satpy/tests/reader_tests/test_fci_l2_nc.py 100.00% <100.00%> (ø)
satpy/readers/scmi.py 82.69% <0.00%> (-0.22%) ⬇️
satpy/readers/electrol_hrit.py 91.40% <0.00%> (-0.20%) ⬇️
satpy/readers/seviri_l1b_hrit.py 90.35% <0.00%> (-0.11%) ⬇️
satpy/readers/hrit_jma.py 97.94% <0.00%> (-0.05%) ⬇️
satpy/modifiers/angles.py 96.70% <0.00%> (ø)
satpy/readers/abi_l2_nc.py 96.00% <0.00%> (ø)
satpy/readers/goes_imager_nc.py 65.46% <0.00%> (ø)
satpy/tests/reader_tests/test_ahi_hrit.py 100.00% <0.00%> (ø)
... and 5 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 c66368c...dc5e03e. Read the comment docs.

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.

I'm good with this, just remove the FIXME tag (you can leave the comment though)

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!

@mraspaud mraspaud merged commit 8efd0a5 into pytroll:main Mar 10, 2022
@djhoese
Copy link
Member

djhoese commented Mar 10, 2022

Can we get an updated title for this PR that describes what was done/fixed so it can be used in release notes?

@strandgren strandgren changed the title Bugfix fci_l2_nc reader Fix dataset attribute typo and reduce amount of categorical dataset filtering in fci_l2_nc reader Mar 10, 2022
@strandgren
Copy link
Collaborator Author

Can we get an updated title for this PR that describes what was done/fixed so it can be used in release notes?

Fixed. Please note that this was a bugfix for a PR merged just a couple of days ago, so it's not a bugfix wrt. the previous release.

@djhoese
Copy link
Member

djhoese commented Mar 10, 2022

Ok thanks for the clarification and the title change. Even if it is just a fix for a previous PR it still shows up in the changelog so nice to have something descriptive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants