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 native resampler not working for some chunk sizes #2291

Merged
merged 7 commits into from Nov 21, 2022

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Nov 20, 2022

This PR fixes the issue described in #2245 where a user's chosen chunk size for their data may not be divisible by the aggregation factor, but the overall shape of the array is. There is no reason except avoiding rechunking that this shouldn't work. This PR makes it so rechunking is done if needed.

I also removed the unnecessary rechunking done in the replication step. My hope is that dask will figure out this rechunking automatically and that this won't be needed.

@simonrp84 any testing you can do would be greatly appreciated.

@djhoese djhoese added the bug label Nov 20, 2022
@djhoese djhoese self-assigned this Nov 20, 2022
@codecov
Copy link

codecov bot commented Nov 20, 2022

Codecov Report

Merging #2291 (95a1f35) into main (402804f) will decrease coverage by 0.00%.
The diff coverage is 99.04%.

@@            Coverage Diff             @@
##             main    #2291      +/-   ##
==========================================
- Coverage   94.32%   94.31%   -0.01%     
==========================================
  Files         308      308              
  Lines       46169    46161       -8     
==========================================
- Hits        43547    43539       -8     
  Misses       2622     2622              
Flag Coverage Δ
behaviourtests 4.62% <8.57%> (+0.01%) ⬆️
unittests 94.95% <99.04%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
satpy/resample.py 79.96% <97.82%> (+0.40%) ⬆️
satpy/tests/test_resample.py 88.90% <100.00%> (-0.37%) ⬇️

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 decreased (-0.0009%) to 94.907% when pulling 95a1f35 on djhoese:bugfix-native-agg-chunks into 402804f on pytroll:main.

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, thanks for also migrating to pytest!

@mraspaud mraspaud merged commit 4ccf5d8 into pytroll:main Nov 21, 2022
@djhoese djhoese deleted the bugfix-native-agg-chunks branch May 17, 2023 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

native resampler fails for some chunk sizes
3 participants