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 some enhancements producing dask arrays wrapped in dask arrays #2190

Merged
merged 2 commits into from
Aug 23, 2022

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Aug 22, 2022

I noticed while counting the number of dask computations that some enhancements were causing more computations than they used to. This was caused by some of the dask arrays producing dask arrays so the dask arrays were being passed to rasterio during geotiff saving which caused yet another compute.

The issue was that due to the refactoring in #2133, the functions were accidentally doing dask "stuff" (delayed functions or da.where) inside a function that was already map_blocks'd. This PR fixes all of that and adds checks to the tests to make sure it hopefully doesn't happen again in the future.

  • Closes #xxxx
  • Tests added
  • Fully documented

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

@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #2190 (ff5ff96) into main (7a23508) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2190   +/-   ##
=======================================
  Coverage   94.01%   94.01%           
=======================================
  Files         289      289           
  Lines       44613    44614    +1     
=======================================
+ Hits        41944    41945    +1     
  Misses       2669     2669           
Flag Coverage Δ
behaviourtests 4.73% <0.00%> (-0.01%) ⬇️
unittests 94.67% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
satpy/enhancements/__init__.py 99.14% <100.00%> (-0.02%) ⬇️
satpy/tests/enhancement_tests/test_enhancements.py 99.64% <100.00%> (+<0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0001%) to 94.624% when pulling ff5ff96 on djhoese:bugfix-btthreshold-enh into 7a23508 on pytroll:main.

@mraspaud mraspaud merged commit a5573c1 into pytroll:main Aug 23, 2022
@djhoese djhoese deleted the bugfix-btthreshold-enh branch November 9, 2022 14:10
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