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 support for multiple values in the DecisionTree used for enhancements #1476

Merged
merged 3 commits into from Dec 14, 2020

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Dec 6, 2020

The current DecisionTree class, which is used as the base class for the EnhancementDecisionTree, currently has issues when a DataArray has multiple sensors specified for .attrs['sensor']. It currently causes a TypeError with a semi-cryptic debug log message produced:

Strange stuff happening in decision tree for %s: %s

This is because the DecisionTree expects a single string value for the "sensor" value, but is getting a set (ex. {'abi', 'glm'}). What this means is that you can never actually configure an enhancement for a DataArray with multiple sensors as it will fail when trying to find the match. This PR is meant to allow for you to both configure an enhancement for sensor: [abi, glm] to match it exactly, or for the provided DataArray with .attrs['sensor'] = {'abi', 'glm'} to match either sensor: abi or sensor: glm.

My one concern about this is that specifying sensor: [abi, glm] in the YAML config will make people think that that matches either abi or glm when it will actually only match the exact pair {'abi', 'glm'}.

  • Tests added
  • Passes flake8 satpy
  • Fully documented

@djhoese djhoese added enhancement code enhancements, features, improvements component:enhancements labels Dec 6, 2020
@djhoese djhoese self-assigned this Dec 6, 2020
@codecov
Copy link

codecov bot commented Dec 6, 2020

Codecov Report

Merging #1476 (1507caf) into master (744728f) will increase coverage by 0.20%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1476      +/-   ##
==========================================
+ Coverage   90.58%   90.78%   +0.20%     
==========================================
  Files         239      241       +2     
  Lines       34313    35111     +798     
==========================================
+ Hits        31081    31876     +795     
- Misses       3232     3235       +3     
Flag Coverage Δ
behaviourtests 4.49% <38.73%> (-0.05%) ⬇️
unittests 91.25% <100.00%> (+0.19%) ⬆️

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

Impacted Files Coverage Δ
satpy/tests/test_writers.py 98.88% <100.00%> (+0.33%) ⬆️
satpy/writers/__init__.py 87.30% <100.00%> (+0.79%) ⬆️
satpy/readers/seviri_l1b_hrit.py 91.90% <0.00%> (-0.28%) ⬇️
satpy/readers/_geos_area.py 100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_abi_l1b.py 100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_geos_area.py 100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_vii_l1b_nc.py 100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_seviri_base.py 100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_fci_l1c_fdhsi.py 100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_seviri_l1b_hrit.py 100.00% <0.00%> (ø)
... and 8 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 744728f...1507caf. Read the comment docs.

@djhoese djhoese changed the title [WIP] Add support for multiple values in the DecisionTree used for enhancements Add support for multiple values in the DecisionTree used for enhancements Dec 7, 2020
@djhoese djhoese marked this pull request as ready for review December 7, 2020 20:16
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 added this to the v0.24.1 milestone Dec 14, 2020
@mraspaud mraspaud merged commit 64fc4ef into pytroll:master Dec 14, 2020
@djhoese djhoese deleted the feature-decision-tree-multival branch December 14, 2020 22:26
@djhoese djhoese mentioned this pull request Dec 14, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:enhancements enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants