-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
ndrolling repr fix #4329
ndrolling repr fix #4329
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the quick fix, @fujiisoup. I'd change the test, otherwise this looks good to me.
This looks good to me. Unfortunately, now that the Lines 236 to 237 in 7e1fbf8
TracebackValueError Traceback (most recent call last)
<ipython-input-38-e6e456ecd183> in <module>
----> 1 rolling_da = r.construct("window_dim", stride=2)
.../xarray/core/rolling.py in construct(self, window_dim, stride, fill_value, **window_dim_kwargs)
277 window_dim = {d: window_dim_kwargs[d] for d in self.dim}
278
--> 279 window_dim = self._mapping_to_list(
280 window_dim, allow_default=False, allow_allsame=False
281 )
.../xarray/core/rolling.py in _mapping_to_list(self, arg, default, allow_default, allow_allsame)
157 return [arg]
158 else:
--> 159 raise ValueError("Mapping argument is necessary.")
160
161
ValueError: Mapping argument is necessary. Edit: what do you think about using this to fix diff --git a/xarray/core/rolling.py b/xarray/core/rolling.py
index 62d14fad..a5f83250 100644
--- a/xarray/core/rolling.py
+++ b/xarray/core/rolling.py
@@ -275,6 +275,8 @@ class DataArrayRolling(Rolling):
"Either window_dim or window_dim_kwargs need to be specified."
)
window_dim = {d: window_dim_kwargs[d] for d in self.dim}
+ elif isinstance(window_dim, str):
+ window_dim = {d: window_dim for d in self.dim}
window_dim = self._mapping_to_list(
window_dim, allow_default=False, allow_allsame=False Edit2: that should be the last docs-only failure related to "rolling", I can build the docs locally with that change. |
Thanks @keewis for checking.
|
normally you'd look at the output of the RTD build, but somehow just the official setup times out instead of printing the log so I put in a support request on RTD. I can reproduce the failure using: In [1]: import xarray as xr
...: import numpy as np
...:
...: arr = xr.DataArray(np.arange(0, 7.5, 0.5).reshape(3, 5), dims=("x", "y"))
...: display(arr)
...: r = arr.rolling(x=2, y=3, min_periods=2)
...: display(r)
...:
...: rolling_da = r.construct("window_dim", stride=2)
...: display(rolling_da)
<xarray.DataArray (x: 3, y: 5)>
array([[0. , 0.5, 1. , 1.5, 2. ],
[2.5, 3. , 3.5, 4. , 4.5],
[5. , 5.5, 6. , 6.5, 7. ]])
Dimensions without coordinates: x, y
DataArrayRolling [x->2,y->3]
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
<ipython-input-1-f9a10c0cfaeb> in <module>
7 display(r)
8
----> 9 rolling_da = r.construct("window_dim", stride=2)
10 display(rolling_da)
11 rolling_da.mean("window_dim", skipna=False)
.../xarray/core/rolling.py in construct(self, window_dim, stride, fill_value, **window_dim_kwargs)
277 window_dim = {d: window_dim_kwargs[d] for d in self.dim}
278
--> 279 window_dim = self._mapping_to_list(
280 window_dim, allow_default=False, allow_allsame=False
281 )
.../xarray/core/rolling.py in _mapping_to_list(self, arg, default, allow_default, allow_allsame)
157 return [arg]
158 else:
--> 159 raise ValueError("Mapping argument is necessary.")
160
161
ValueError: Mapping argument is necessary. so the bug is in |
Thanks, @keewis , for the clarification. It was a bug in the documentation page but not in It should raise an error in this case, because for 2d rolling we need 2 dimension names, rolling_da = r.construct(x="x_win", y='y_win' , stride=2) I corrected the documentation and error message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick response, @fujiisoup. Looks good to me. I think this should be ready to go once CI passes.
thanks, that fixed the build on my own RTD setup, but the main setup is quite a bit slower... Edit: it passed on the main setup, too. Let's merge on green. |
all CI pass. Thanks for the quick fixes, @fujiisoup |
* 'master' of github.com:pydata/xarray: (260 commits) Increase support window of all dependencies (pydata#4296) Implement interp for interpolating between chunks of data (dask) (pydata#4155) Add @mathause to current core developers. (pydata#4335) install sphinx-autosummary-accessors from conda-forge (pydata#4332) Use sphinx-accessors-autosummary (pydata#4323) ndrolling fixes (pydata#4329) DOC: fix typo argmin -> argmax in DataArray.argmax docstring (pydata#4327) pin sphinx to 3.1(pydata#4326) nd-rolling (pydata#4219) Implicit dask import 4164 (pydata#4318) allow customizing the inline repr of a duck array (pydata#4248) silence the known docs CI issues (pydata#4316) enh: fixed pydata#4302 (pydata#4315) Remove all unused and warn-raising methods from AbstractDataStore (pydata#4310) Fix map_blocks example (pydata#4305) Fix docstring for missing_dims argument to isel methods (pydata#4298) Support for PyCharm remote deployment (pydata#4299) Update map_blocks and map_overlap docstrings (pydata#4303) Lazily load resource files (pydata#4297) warn about the removal of the ufuncs (pydata#4268) ...
* upstream/master: (34 commits) Fix bug in computing means of cftime.datetime arrays (pydata#4344) fix some str accessor inconsistencies (pydata#4339) pin matplotlib in ci/requirements/doc.yml (pydata#4340) Clarify drop_vars return value. (pydata#4244) Support explicitly setting a dimension order with to_dataframe() (pydata#4333) Increase support window of all dependencies (pydata#4296) Implement interp for interpolating between chunks of data (dask) (pydata#4155) Add @mathause to current core developers. (pydata#4335) install sphinx-autosummary-accessors from conda-forge (pydata#4332) Use sphinx-accessors-autosummary (pydata#4323) ndrolling fixes (pydata#4329) DOC: fix typo argmin -> argmax in DataArray.argmax docstring (pydata#4327) pin sphinx to 3.1(pydata#4326) nd-rolling (pydata#4219) Implicit dask import 4164 (pydata#4318) allow customizing the inline repr of a duck array (pydata#4248) silence the known docs CI issues (pydata#4316) enh: fixed pydata#4302 (pydata#4315) Remove all unused and warn-raising methods from AbstractDataStore (pydata#4310) Fix map_blocks example (pydata#4305) ...
* upstream/master: (40 commits) Fix bug in computing means of cftime.datetime arrays (pydata#4344) fix some str accessor inconsistencies (pydata#4339) pin matplotlib in ci/requirements/doc.yml (pydata#4340) Clarify drop_vars return value. (pydata#4244) Support explicitly setting a dimension order with to_dataframe() (pydata#4333) Increase support window of all dependencies (pydata#4296) Implement interp for interpolating between chunks of data (dask) (pydata#4155) Add @mathause to current core developers. (pydata#4335) install sphinx-autosummary-accessors from conda-forge (pydata#4332) Use sphinx-accessors-autosummary (pydata#4323) ndrolling fixes (pydata#4329) DOC: fix typo argmin -> argmax in DataArray.argmax docstring (pydata#4327) pin sphinx to 3.1(pydata#4326) nd-rolling (pydata#4219) Implicit dask import 4164 (pydata#4318) allow customizing the inline repr of a duck array (pydata#4248) silence the known docs CI issues (pydata#4316) enh: fixed pydata#4302 (pydata#4315) Remove all unused and warn-raising methods from AbstractDataStore (pydata#4310) Fix map_blocks example (pydata#4305) ...
isort . && black . && mypy . && flake8
There was a bug in
rolling.__repr__
but it was not tested.Fixed and tests are added.