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 the dataid sorting #1556

Merged
merged 1 commit into from
Feb 20, 2021
Merged

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Feb 20, 2021

When the query and the dataids to be sorted have a different set of keys, the sorting would sometimes not go through all the keys before returning. We fix this by converting a spurious break into a continue. This fixes #1549.

When the query and the dataids to be sorted have a different set of keys, the sorting would sometimes not go through all the tests before returning. We fix this by converting a spurious `break` into a `continue`. This fixes pytroll#1549.
@mraspaud mraspaud added bug component:dep_tree Dependency tree and dataset loading labels Feb 20, 2021
@mraspaud mraspaud self-assigned this Feb 20, 2021
@mraspaud mraspaud requested a review from djhoese as a code owner February 20, 2021 07:47
@codecov
Copy link

codecov bot commented Feb 20, 2021

Codecov Report

Merging #1556 (143304c) into master (f863bc2) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1556      +/-   ##
==========================================
+ Coverage   92.15%   92.18%   +0.02%     
==========================================
  Files         251      251              
  Lines       36764    36777      +13     
==========================================
+ Hits        33881    33902      +21     
+ Misses       2883     2875       -8     
Flag Coverage Δ
behaviourtests 4.49% <0.00%> (-0.01%) ⬇️
unittests 92.71% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
satpy/dataset/dataid.py 94.05% <100.00%> (ø)
satpy/tests/test_dataset.py 100.00% <100.00%> (ø)
satpy/tests/reader_tests/test_vii_base_nc.py 98.73% <0.00%> (+1.26%) ⬆️
satpy/tests/reader_tests/test_nwcsaf_msg.py 98.31% <0.00%> (+1.68%) ⬆️
satpy/tests/reader_tests/test_vii_l1b_nc.py 100.00% <0.00%> (+2.81%) ⬆️
satpy/tests/reader_tests/test_vii_l2_nc.py 100.00% <0.00%> (+6.06%) ⬆️

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 f863bc2...143304c. Read the comment docs.

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.

LGTM just to make sure, there is a break a couple lines below your change, does this need to change?

@mraspaud
Copy link
Member Author

The one after is fine, as the distance becomes infinite, there is no point in continuing.

@mraspaud mraspaud merged commit 1aa9c5f into pytroll:master Feb 20, 2021
@mraspaud mraspaud deleted the fix-dataid-sorting branch February 20, 2021 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component:dep_tree Dependency tree and dataset loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Satpy problems with MODIS
2 participants