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 DataArrayRolling.__iter__ with center=True #6744

Merged
merged 7 commits into from
Jul 14, 2022

Conversation

headtr1ck
Copy link
Collaborator

@headtr1ck headtr1ck commented Jul 2, 2022

I have taken the freedom to move all rolling related tests into their own testing module.
#6730 should then take care of the (by now) copy-pasted da and ds fixtures.

xarray/core/rolling.py Outdated Show resolved Hide resolved
@headtr1ck
Copy link
Collaborator Author

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
Copy link
Collaborator Author

headtr1ck commented Jul 2, 2022

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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

@dcherian
Copy link
Contributor

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Contributor

Thanks @headtr1ck

@dcherian dcherian merged commit f28d7f8 into pydata:main Jul 14, 2022
@headtr1ck headtr1ck deleted the rolliter branch July 18, 2022 15:31
dcherian added a commit to keewis/xarray that referenced this pull request Jul 22, 2022
* main: (313 commits)
  Update whats-new
  Release notes for v2022.06.0 (pydata#6815)
  Drop multi-indexes when assigning to a multi-indexed variable (pydata#6798)
  Support NumPy array API (experimental) (pydata#6804)
  Add cumsum to DatasetGroupBy (pydata#6525)
  Refactor groupby binary ops code. (pydata#6789)
  Update DataArray.rename + docu (pydata#6665)
  Switch to T_DataArray and T_Dataset in concat (pydata#6784)
  Fix typos found by codespell (pydata#6794)
  Update groupby attrs tests (pydata#6787)
  Update map_blocks to use chunksizes property. (pydata#6776)
  Fix `DataArrayRolling.__iter__` with `center=True` (pydata#6744)
  [test-upstream] Update flox repo URL (pydata#6780)
  Move _infer_meta_data and _parse_size to utils (pydata#6779)
  Make the `sel` error more descriptive when `method` is unset (pydata#6774)
  Move Rolling tests to their own testing module (pydata#6777)
  [pre-commit.ci] pre-commit autoupdate (pydata#6773)
  move da and ds fixtures to conftest.py (pydata#6730)
  Bump EnricoMi/publish-unit-test-result-action from 1 to 2 (pydata#6770)
  Type shape methods (pydata#6767)
  ...
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.

"center" kwarg ignored when manually iterating over DataArrayRolling
3 participants