Fix DataArrayRolling.__iter__ with center=True by headtr1ck · Pull Request #6744 · pydata/xarray (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation9 Commits7 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

headtr1ck

Illviljan

@headtr1ck

I am now trying to get this to work for Datasets as well.
But the check of min_periods fails for variables that do not contain the rolling dim, since the count is 1.
Any idea how to solve this issue?

Is there a way to get a boolean, scalar DataArray/set that contains this information?

@headtr1ck

Would it be useful to add a function has_dim that returns e.g.:

xr.DataArray([], dims="x").has_dim("x")  #-> xr.DataArray(True)
xr.Dataset({"x": ("x", []), "y": ("y", [])}).has_dim("x")  # -> xr.Dataset({"x": True, "y": False})

?
Since these are scalar (no dims) they are nicely broadcastable and can be used in xarray pipelines.

But maybe this already works easily and I just don't know about it?

@headtr1ck

@headtr1ck

But maybe that is already too complicated.

Anyone has an idea how to get this:

counts = window.count(dim=self.dim[0])
window = window.where(counts >= self.min_periods)

operate only on variables that contain self.dim[0]?
If possible in a way that works both on DataArrays and Datasets :)

@headtr1ck

@headtr1ck

Btw: this issue #6749 is the reason why it currently does not work for datasets.

@headtr1ck

@dcherian

Would you mind moving the first commit to a new PR that we can merge quickly please? It'll be easier to see any new tests you've added then

@headtr1ck

Would you mind moving the first commit to a new PR that we can merge quickly please? It'll be easier to see any new tests you've added then

I'm fast in adding scope kreep xD
Done in #6777

@headtr1ck

@headtr1ck

We could merge this PR after a final review.
For now it fixes the bug in the issue.

For a DatasetRolling.iter support we could either open a new issue or just wait until someone requests it :)

@dcherian

dcherian added a commit to keewis/xarray that referenced this pull request

Jul 22, 2022

@dcherian