From eb6b42c80c74a28a19df54f39453f4a278104c42 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 15 Oct 2019 20:36:47 -0600 Subject: [PATCH 01/12] Deprecate allow_lazy --- xarray/core/common.py | 17 ++++------------- xarray/core/dataset.py | 2 +- xarray/core/groupby.py | 4 +--- xarray/core/variable.py | 13 ++++++++++++- xarray/tests/test_variable.py | 4 ++++ 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/xarray/core/common.py b/xarray/core/common.py index 45d860a1797..a80eb06ba38 100644 --- a/xarray/core/common.py +++ b/xarray/core/common.py @@ -42,14 +42,12 @@ def _reduce_method(cls, func: Callable, include_skipna: bool, numeric_only: bool if include_skipna: def wrapped_func(self, dim=None, axis=None, skipna=None, **kwargs): - return self.reduce( - func, dim, axis, skipna=skipna, allow_lazy=True, **kwargs - ) + return self.reduce(func, dim, axis, skipna=skipna, **kwargs) else: def wrapped_func(self, dim=None, axis=None, **kwargs): # type: ignore - return self.reduce(func, dim, axis, allow_lazy=True, **kwargs) + return self.reduce(func, dim, axis, **kwargs) return wrapped_func @@ -82,20 +80,13 @@ def _reduce_method(cls, func: Callable, include_skipna: bool, numeric_only: bool def wrapped_func(self, dim=None, skipna=None, **kwargs): return self.reduce( - func, - dim, - skipna=skipna, - numeric_only=numeric_only, - allow_lazy=True, - **kwargs, + func, dim, skipna=skipna, numeric_only=numeric_only, **kwargs ) else: def wrapped_func(self, dim=None, **kwargs): # type: ignore - return self.reduce( - func, dim, numeric_only=numeric_only, allow_lazy=True, **kwargs - ) + return self.reduce(func, dim, numeric_only=numeric_only, **kwargs) return wrapped_func diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 3fdde8fa4e3..647b338514c 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -3999,7 +3999,7 @@ def reduce( keep_attrs: bool = None, keepdims: bool = False, numeric_only: bool = False, - allow_lazy: bool = False, + allow_lazy: bool = None, **kwargs: Any, ) -> "Dataset": """Reduce this dataset by applying `func` along some dimension(s). diff --git a/xarray/core/groupby.py b/xarray/core/groupby.py index 52eb17df18d..b1ddc4ba1fe 100644 --- a/xarray/core/groupby.py +++ b/xarray/core/groupby.py @@ -564,9 +564,7 @@ def _first_or_last(self, op, skipna, keep_attrs): return self._obj if keep_attrs is None: keep_attrs = _get_keep_attrs(default=True) - return self.reduce( - op, self._group_dim, skipna=skipna, keep_attrs=keep_attrs, allow_lazy=True - ) + return self.reduce(op, self._group_dim, skipna=skipna, keep_attrs=keep_attrs) def first(self, skipna=None, keep_attrs=None): """Return the first element of each group along the group dimension diff --git a/xarray/core/variable.py b/xarray/core/variable.py index 37672cd82d9..917d432495f 100644 --- a/xarray/core/variable.py +++ b/xarray/core/variable.py @@ -1,5 +1,6 @@ import functools import itertools +import warnings from collections import defaultdict from datetime import timedelta from distutils.version import LooseVersion @@ -1416,7 +1417,7 @@ def reduce( axis=None, keep_attrs=None, keepdims=False, - allow_lazy=False, + allow_lazy=None, **kwargs, ): """Reduce this array by applying `func` along some dimension(s). @@ -1457,7 +1458,17 @@ def reduce( if dim is not None: axis = self.get_axis_num(dim) + + if allow_lazy is not None: + warnings.warn( + "allow_lazy will be deprecated in version 0.16.0. It is now True by default.", + DeprecationWarning, + ) + else: + allow_lazy = True + input_data = self.data if allow_lazy else self.values + if axis is not None: data = func(input_data, axis=axis, **kwargs) else: diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index 78723eda013..4b6b23904ea 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -1474,6 +1474,10 @@ def test_reduce(self): with raises_regex(ValueError, "cannot supply both"): v.mean(dim="x", axis=0) + with pytest.warns(DeprecationWarning, match="allow_lazy will be deprecated"): + v.mean("x", allow_lazy=True) + with pytest.warns(DeprecationWarning, match="allow_lazy will be deprecated"): + v.mean("x", allow_lazy=False) def test_quantile(self): v = Variable(["x", "y"], self.d) From 9d85efe7e50b6c1e7e6d7db371d05a41fd11cc6d Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 22 Oct 2019 15:43:21 -0600 Subject: [PATCH 02/12] add whats-new --- doc/whats-new.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 0f4d0c10f1f..a43aedb1f3c 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -44,6 +44,9 @@ Bug fixes - Sync with cftime by removing `dayofwk=-1` for cftime>=1.0.4. By `Anderson Banihirwe `_. +- Rolling reduction operations now longer compute dask arrays by default. (:issue:3161). + In addition, the ``allow_lazy`` kwarg to ``reduce`` is deprecated. + By `Deepak Cherian `_. Documentation ~~~~~~~~~~~~~ From 0e24c89defe0022976f014b2602957c4c91780f7 Mon Sep 17 00:00:00 2001 From: dcherian Date: Thu, 7 Nov 2019 17:49:51 -0700 Subject: [PATCH 03/12] test that reductions are lazy --- xarray/tests/test_dask.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/xarray/tests/test_dask.py b/xarray/tests/test_dask.py index ae8f43cb66d..c2b9fd4954a 100644 --- a/xarray/tests/test_dask.py +++ b/xarray/tests/test_dask.py @@ -211,6 +211,8 @@ def test_reduce(self): self.assertLazyAndAllClose((u < 1).all("x"), (v < 1).all("x")) with raises_regex(NotImplementedError, "dask"): v.median() + with raise_if_dask_computes(): + v.reduce(np.mean) def test_missing_values(self): values = np.array([0, 1, np.nan, 3]) @@ -436,7 +438,25 @@ def test_groupby(self): v = self.lazy_array expected = u.groupby("x").mean(xr.ALL_DIMS) - actual = v.groupby("x").mean(xr.ALL_DIMS) + with raise_if_dask_computes(): + actual = v.groupby("x").mean(xr.ALL_DIMS) + self.assertLazyAndAllClose(expected, actual) + + with raise_if_dask_computes(): + actual = v.groupby("x").reduce(np.nanmean, xr.ALL_DIMS) + self.assertLazyAndAllClose(expected, actual) + + def test_rolling(self): + u = self.eager_array + v = self.lazy_array + + expected = u.rolling(x=2).mean() + with raise_if_dask_computes(): + actual = v.rolling(x=2).mean() + self.assertLazyAndAllClose(expected, actual) + + with raise_if_dask_computes(): + actual = v.rolling(x=2).reduce(np.nanmean) self.assertLazyAndAllClose(expected, actual) def test_groupby_first(self): @@ -448,7 +468,8 @@ def test_groupby_first(self): with raises_regex(NotImplementedError, "dask"): v.groupby("ab").first() expected = u.groupby("ab").first() - actual = v.groupby("ab").first(skipna=False) + with raise_if_dask_computes(): + actual = v.groupby("ab").first(skipna=False) self.assertLazyAndAllClose(expected, actual) def test_reindex(self): From aadd9220750f53dd05e3acb4e7c702cf1681d850 Mon Sep 17 00:00:00 2001 From: dcherian Date: Thu, 7 Nov 2019 17:52:21 -0700 Subject: [PATCH 04/12] minor whats-new fix. --- doc/whats-new.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index cdb920c721c..d4aa5adeddd 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -44,7 +44,7 @@ Bug fixes - Sync with cftime by removing `dayofwk=-1` for cftime>=1.0.4. By `Anderson Banihirwe `_. -- Rolling reduction operations now longer compute dask arrays by default. (:issue:3161). +- Rolling reduction operations now longer compute dask arrays by default. (:issue:`3161`). In addition, the ``allow_lazy`` kwarg to ``reduce`` is deprecated. By `Deepak Cherian `_. From 9bf1bf66394642e80e9ca6c4e7eddf3bc162f0e8 Mon Sep 17 00:00:00 2001 From: dcherian Date: Fri, 8 Nov 2019 08:35:39 -0700 Subject: [PATCH 05/12] fix merge wahts=new --- doc/whats-new.rst | 4 ---- 1 file changed, 4 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 891a4233c19..27ccd7ed189 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -81,13 +81,9 @@ Bug fixes By `Deepak Cherian `_. - Sync with cftime by removing `dayofwk=-1` for cftime>=1.0.4. By `Anderson Banihirwe `_. - -- Rolling reduction operations now longer compute dask arrays by default. (:issue:`3161`). - - Rolling reduction operations now longer compute dask arrays by default. (:issue:3161). In addition, the ``allow_lazy`` kwarg to ``reduce`` is deprecated. By `Deepak Cherian `_. - - Fix :py:meth:`xarray.core.groupby.DataArrayGroupBy.reduce` and :py:meth:`xarray.core.groupby.DatasetGroupBy.reduce` when reducing over multiple dimensions. (:issue:`3402`). By `Deepak Cherian `_ From 67d671402f0c5a47e2b8cd23578901d4b54b1ee8 Mon Sep 17 00:00:00 2001 From: dcherian Date: Fri, 8 Nov 2019 11:19:53 -0700 Subject: [PATCH 06/12] fix bad merge. --- xarray/tests/test_dask.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xarray/tests/test_dask.py b/xarray/tests/test_dask.py index 85b9e530dc6..6c874d26c1d 100644 --- a/xarray/tests/test_dask.py +++ b/xarray/tests/test_dask.py @@ -494,9 +494,9 @@ def test_groupby(self): actual = v.groupby("x").mean(...) self.assertLazyAndAllClose(expected, actual) - actual = v.groupby("x").reduce(np.nanmean, ...) with raise_if_dask_computes(): - self.assertLazyAndAllClose(expected, actual) + actual = v.groupby("x").reduce(np.nanmean, ...) + self.assertLazyAndAllClose(expected, actual) def test_rolling(self): u = self.eager_array From f96802e5191b930be3a3c18dbd95d3a20ccee415 Mon Sep 17 00:00:00 2001 From: dcherian Date: Fri, 8 Nov 2019 11:22:28 -0700 Subject: [PATCH 07/12] remove tests that only work with nep-18 --- xarray/tests/test_dask.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/xarray/tests/test_dask.py b/xarray/tests/test_dask.py index 6c874d26c1d..b3995e4b1ba 100644 --- a/xarray/tests/test_dask.py +++ b/xarray/tests/test_dask.py @@ -494,10 +494,6 @@ def test_groupby(self): actual = v.groupby("x").mean(...) self.assertLazyAndAllClose(expected, actual) - with raise_if_dask_computes(): - actual = v.groupby("x").reduce(np.nanmean, ...) - self.assertLazyAndAllClose(expected, actual) - def test_rolling(self): u = self.eager_array v = self.lazy_array @@ -507,10 +503,6 @@ def test_rolling(self): actual = v.rolling(x=2).mean() self.assertLazyAndAllClose(expected, actual) - with raise_if_dask_computes(): - actual = v.rolling(x=2).reduce(np.nanmean) - self.assertLazyAndAllClose(expected, actual) - def test_groupby_first(self): u = self.eager_array v = self.lazy_array From 7f1e1298f15abad4d20d2fbb6c08f4750ee274fe Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Tue, 12 Nov 2019 15:37:34 +0000 Subject: [PATCH 08/12] Update doc/whats-new.rst Co-Authored-By: Mathias Hauser --- doc/whats-new.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 27ccd7ed189..10b73a955b0 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -81,7 +81,7 @@ Bug fixes By `Deepak Cherian `_. - Sync with cftime by removing `dayofwk=-1` for cftime>=1.0.4. By `Anderson Banihirwe `_. -- Rolling reduction operations now longer compute dask arrays by default. (:issue:3161). +- Rolling reduction operations no longer compute dask arrays by default. (:issue:3161). In addition, the ``allow_lazy`` kwarg to ``reduce`` is deprecated. By `Deepak Cherian `_. - Fix :py:meth:`xarray.core.groupby.DataArrayGroupBy.reduce` and From 40e03a69df26f552284edb1853f22d3c00c6ac4b Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Tue, 12 Nov 2019 15:37:41 +0000 Subject: [PATCH 09/12] Update xarray/core/variable.py Co-Authored-By: Mathias Hauser --- xarray/core/variable.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/core/variable.py b/xarray/core/variable.py index 363f8368066..a5e38d6e35e 100644 --- a/xarray/core/variable.py +++ b/xarray/core/variable.py @@ -1470,7 +1470,7 @@ def reduce( if allow_lazy is not None: warnings.warn( - "allow_lazy will be deprecated in version 0.16.0. It is now True by default.", + "allow_lazy is deprecated and will be removed in version 0.16.0. It is now True by default.", DeprecationWarning, ) else: From d8faed284c1b65c869a925bd2be390b030698ad8 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 12 Nov 2019 08:38:19 -0700 Subject: [PATCH 10/12] fix whats-new --- doc/whats-new.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 10b73a955b0..ce4bf6f35f8 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -81,7 +81,7 @@ Bug fixes By `Deepak Cherian `_. - Sync with cftime by removing `dayofwk=-1` for cftime>=1.0.4. By `Anderson Banihirwe `_. -- Rolling reduction operations no longer compute dask arrays by default. (:issue:3161). +- Rolling reduction operations no longer compute dask arrays by default. (:issue:`3161`). In addition, the ``allow_lazy`` kwarg to ``reduce`` is deprecated. By `Deepak Cherian `_. - Fix :py:meth:`xarray.core.groupby.DataArrayGroupBy.reduce` and From 97bb43cfa9f45da35960ceeb12eae9fccae04c12 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 12 Nov 2019 08:41:13 -0700 Subject: [PATCH 11/12] Fix test that assumed NEP-18 --- xarray/tests/test_dask.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xarray/tests/test_dask.py b/xarray/tests/test_dask.py index b3995e4b1ba..698e9084f0b 100644 --- a/xarray/tests/test_dask.py +++ b/xarray/tests/test_dask.py @@ -12,6 +12,7 @@ import xarray as xr import xarray.ufuncs as xu from xarray import DataArray, Dataset, Variable +from xarray.core import duck_array_ops from xarray.testing import assert_chunks_equal from xarray.tests import mock @@ -218,7 +219,7 @@ def test_reduce(self): with raises_regex(NotImplementedError, "dask"): v.median() with raise_if_dask_computes(): - v.reduce(np.mean) + v.reduce(duck_array_ops.mean) def test_missing_values(self): values = np.array([0, 1, np.nan, 3]) From debc76627ee1468958ae78c0cc665cfd99cc3343 Mon Sep 17 00:00:00 2001 From: dcherian Date: Tue, 12 Nov 2019 08:57:37 -0700 Subject: [PATCH 12/12] fix tests. --- xarray/tests/test_variable.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/xarray/tests/test_variable.py b/xarray/tests/test_variable.py index 763529e1d6f..d394919dbdd 100644 --- a/xarray/tests/test_variable.py +++ b/xarray/tests/test_variable.py @@ -1477,10 +1477,10 @@ def test_reduce(self): with raises_regex(ValueError, "cannot supply both"): v.mean(dim="x", axis=0) - with pytest.warns(DeprecationWarning, match="allow_lazy will be deprecated"): - v.mean("x", allow_lazy=True) - with pytest.warns(DeprecationWarning, match="allow_lazy will be deprecated"): - v.mean("x", allow_lazy=False) + with pytest.warns(DeprecationWarning, match="allow_lazy is deprecated"): + v.mean(dim="x", allow_lazy=True) + with pytest.warns(DeprecationWarning, match="allow_lazy is deprecated"): + v.mean(dim="x", allow_lazy=False) def test_quantile(self): v = Variable(["x", "y"], self.d)