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

Improve handling of dependency loading when reader has multiple matches #474

Merged
merged 3 commits into from
Oct 24, 2018

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Oct 24, 2018

This is to address issues brought up related to #458 where scn.available_composite_names() would result in an AssertionError instead of the proper result. It now shows me only 'overview', does that sound right @mraspaud?

This PR adds a subclass of KeyError to the DatasetDict key checks called TooManyResults to distinguish itself from a regular KeyError where the key doesn't exist. Since it is a subclass of KeyError it shouldn't cause any issues with other code that was specifically catching KeyError.

Note: This is technically a workaround because SatPy isn't smart enough right now to handle datasets with different names but the same exact wavelength.

This PR also sits on top of #458.

  • Tests added
  • Tests passed
  • Passes git diff origin/master -- "*py" | flake8 --diff

@djhoese djhoese added enhancement code enhancements, features, improvements component:readers component:dep_tree Dependency tree and dataset loading labels Oct 24, 2018
@djhoese djhoese self-assigned this Oct 24, 2018
@djhoese djhoese requested a review from mraspaud October 24, 2018 02:32
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 73.957% when pulling 962ac5b on djhoese:bugfix-too-many-keys into 46e86d7 on pytroll:master.

@codecov
Copy link

codecov bot commented Oct 24, 2018

Codecov Report

Merging #474 into master will increase coverage by 0.06%.
The diff coverage is 96.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #474      +/-   ##
==========================================
+ Coverage   73.89%   73.95%   +0.06%     
==========================================
  Files         136      136              
  Lines       17909    17959      +50     
==========================================
+ Hits        13234    13282      +48     
- Misses       4675     4677       +2
Impacted Files Coverage Δ
satpy/readers/__init__.py 94.11% <100%> (+0.04%) ⬆️
satpy/node.py 94.28% <100%> (+0.62%) ⬆️
satpy/tests/test_scene.py 99.32% <100%> (ø) ⬆️
satpy/utils.py 62.71% <77.77%> (+1.24%) ⬆️

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 46e86d7...962ac5b. 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.

lgtm. This absorbs #458, right ?

@djhoese
Copy link
Member Author

djhoese commented Oct 24, 2018

This PR also sits on top of #458.

Yes.

@djhoese djhoese merged commit be0c391 into pytroll:master Oct 24, 2018
@djhoese djhoese deleted the bugfix-too-many-keys branch October 24, 2018 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:dep_tree Dependency tree and dataset loading component:readers enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants