Skip to content

Fixing non-lazy behavior of sampled+weighted#4668

Merged
dcherian merged 8 commits into
pydata:masterfrom
jbusecke:lazy_resample_weighted_mean
Dec 16, 2020
Merged

Fixing non-lazy behavior of sampled+weighted#4668
dcherian merged 8 commits into
pydata:masterfrom
jbusecke:lazy_resample_weighted_mean

Conversation

@jbusecke

@jbusecke jbusecke commented Dec 9, 2020

Copy link
Copy Markdown

@jbusecke jbusecke changed the title Added tests with resample+weighted, partially failing Fixing non-lazy behavior of sampled+weighted Dec 9, 2020

@mathause mathause left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. You added the @requires_cftime to the wrong test.

@jbusecke

Copy link
Copy Markdown
Author

Oops. Thanks for catching that. Should be fixed now.

@mathause

Copy link
Copy Markdown
Collaborator

👍 do you wan't to add a what's new entry? In any case we can merge this in a few days unless someone else has a comment.

@jbusecke

Copy link
Copy Markdown
Author

do you wan't to add a what's new entry? In any case we can merge this in a few days unless someone else has a comment.

Not sure. Would you think that this is significant enough? If yes, Id be happy to do it.

@mathause

Copy link
Copy Markdown
Collaborator

Up to you. Maybe under internal changes.

@keewis

keewis commented Dec 14, 2020

Copy link
Copy Markdown
Collaborator

I think #4625 is a bug, so "bug fixes" would probably be better

Comment thread xarray/tests/test_weighted.py Outdated
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>

@dcherian dcherian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @jbusecke

It'd be nice to understand why dask was making a deep copy, but the fix looks good.

@jbusecke

Copy link
Copy Markdown
Author

Thanks for all the help! I made the entry to whats-new.rst.

@dcherian dcherian merged commit 778a16e into pydata:master Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non lazy behavior for weighted average when using resampled data

4 participants