Future warning for default reduction dimension of groupby

doc/whats-new.rst

@@ -30,6 +30,14 @@ What's New
 v0.10.9 (unreleased)
 --------------------
 
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+- Reduction of :py:meth:`DataArray.groupby` and :py:meth:`DataArray.resample`
+  without dimension argument will change in the next release.
+  Now we warn a FutureWarning.
+  By `Keisuke Fujii <https://github.com/fujiisoup>`_.
+
 Documentation
 ~~~~~~~~~~~~~
 

xarray/__init__.py

@@ -33,3 +33,5 @@
 from . import tutorial
 from . import ufuncs
 from . import testing
+
+from .core.common import ALL_DIMS

xarray/core/common.py

@@ -9,7 +9,11 @@
 from . import duck_array_ops, dtypes, formatting, ops
 from .arithmetic import SupportsArithmetic
 from .pycompat import OrderedDict, basestring, dask_array_type, suppress
-from .utils import Frozen, SortedKeysDict
+from .utils import Frozen, SortedKeysDict, ReprObject
+
+
+# Use as a sentinel value to indicate a all dimensions, which
+ALL_DIMS = ReprObject('<all-dims>')
 
 
 class ImplementsArrayReduce(object):

xarray/core/dataset.py

@@ -18,7 +18,8 @@
 from .. import conventions
 from .alignment import align
 from .common import (
-    DataWithCoords, ImplementsDatasetReduce, _contains_datetime_like_objects)
+    ALL_DIMS, DataWithCoords, ImplementsDatasetReduce,
+    _contains_datetime_like_objects)
 from .coordinates import (
     DatasetCoordinates, Indexes, LevelCoordinatesSource,
     assert_coordinate_consistent, remap_label_indexers)
@@ -2722,7 +2723,7 @@ def combine_first(self, other):
         out = ops.fillna(self, other, join="outer", dataset_join="outer")
         return out
 
-    def reduce(self, func, dim=None, keep_attrs=False, numeric_only=False,
+    def reduce(self, func, dim=None, keep_attrs=False, numeric_only=False, 
                allow_lazy=False, **kwargs):
         """Reduce this dataset by applying `func` along some dimension(s).
 
@@ -2750,6 +2751,8 @@ def reduce(self, func, dim=None, keep_attrs=False, numeric_only=False,
             Dataset with this object's DataArrays replaced with new DataArrays
             of summarized data and the indicated dimension(s) removed.
         """
+        if dim == ALL_DIMS:
+            dim = None
         if isinstance(dim, basestring):
             dims = set([dim])
         elif dim is None:

xarray/core/groupby.py

@@ -1,14 +1,15 @@
 from __future__ import absolute_import, division, print_function
 
 import functools
+import warnings
 
 import numpy as np
 import pandas as pd
 
-from . import dtypes, duck_array_ops, nputils, ops
+from . import dtypes, duck_array_ops, nputils, ops, utils
 from .arithmetic import SupportsArithmetic
 from .combine import concat
-from .common import ImplementsArrayReduce, ImplementsDatasetReduce
+from .common import ALL_DIMS, ImplementsArrayReduce, ImplementsDatasetReduce
 from .pycompat import integer_types, range, zip
 from .utils import hashable, maybe_wrap_array, peek_at, safe_cast_to_index
 from .variable import IndexVariable, Variable, as_variable
@@ -567,10 +568,41 @@ def reduce(self, func, dim=None, axis=None, keep_attrs=False,
             Array with summarized data and the indicated dimension(s)
             removed.
         """
+        if dim == DEFAULT_DIMS:
+            dim = ALL_DIMS
+            # TODO change this to dim = self._group_dim after
+            # the deprecation process
+            if self._obj.ndim > 1:
+                warnings.warn(
+                    "Default reduction dimension will be changed to the "
+                    "grouped dimension after xarray 0.11. To silence this "
+                    "warning, pass dim=xarray.ALL_DIMS explicitly.",
+                    FutureWarning, stacklevel=2)
+        elif dim is None:
+            dim = self._group_dim
+
         def reduce_array(ar):
             return ar.reduce(func, dim, axis, keep_attrs=keep_attrs, **kwargs)
         return self.apply(reduce_array, shortcut=shortcut)
 
+    # TODO remove the following class method and DEFAULT_DIMS after the
+    # deprecation cycle
+    @classmethod
+    def _reduce_method(cls, func, include_skipna, numeric_only):
+        if include_skipna:
+            def wrapped_func(self, dim=DEFAULT_DIMS, axis=None, skipna=None,
+                             keep_attrs=False, **kwargs):
+                return self.reduce(func, dim, axis, keep_attrs=keep_attrs,
+                                   skipna=skipna, allow_lazy=True, **kwargs)
+        else:
+            def wrapped_func(self, dim=DEFAULT_DIMS, axis=None,
+                             keep_attrs=False, **kwargs):
+                return self.reduce(func, dim, axis, keep_attrs=keep_attrs,
+                                   allow_lazy=True, **kwargs)
+        return wrapped_func
+
+
+DEFAULT_DIMS = utils.ReprObject('<default-dims>')
 
 ops.inject_reduce_methods(DataArrayGroupBy)
 ops.inject_binary_ops(DataArrayGroupBy)
@@ -649,10 +681,40 @@ def reduce(self, func, dim=None, keep_attrs=False, **kwargs):
             Array with summarized data and the indicated dimension(s)
             removed.
         """
+        if dim == DEFAULT_DIMS:
+            dim = ALL_DIMS
+            # TODO change this to dim = self._group_dim after
+            # the deprecation process. Do not forget to remove _reduce_method
+            warnings.warn(
+                "Default reduction dimension will be changed to the "
+                "grouped dimension after xarray 0.11. To silence this "
+                "warning, pass dim=xarray.ALL_DIMS explicitly.",
+                FutureWarning, stacklevel=2)
+        elif dim is None:
+            dim = self._group_dim
+
         def reduce_dataset(ds):
             return ds.reduce(func, dim, keep_attrs, **kwargs)
         return self.apply(reduce_dataset)
 
+    # TODO remove the following class method and DEFAULT_DIMS after the
+    # deprecation cycle
+    @classmethod
+    def _reduce_method(cls, func, include_skipna, numeric_only):
+        if include_skipna:
+            def wrapped_func(self, dim=DEFAULT_DIMS, keep_attrs=False,
+                             skipna=None, **kwargs):
+                return self.reduce(func, dim, keep_attrs, skipna=skipna,
+                                   numeric_only=numeric_only, allow_lazy=True,
+                                   **kwargs)
+        else:
+            def wrapped_func(self, dim=DEFAULT_DIMS, keep_attrs=False,
+                             **kwargs):
+                return self.reduce(func, dim, keep_attrs,
+                                   numeric_only=numeric_only, allow_lazy=True,
+                                   **kwargs)
+        return wrapped_func
+
     def assign(self, **kwargs):
         """Assign data variables by group.
 

xarray/core/resample.py

@@ -1,7 +1,7 @@
 from __future__ import absolute_import, division, print_function
 
 from . import ops
-from .groupby import DataArrayGroupBy, DatasetGroupBy
+from .groupby import DataArrayGroupBy, DatasetGroupBy, DEFAULT_DIMS
 from .pycompat import OrderedDict, dask_array_type
 
 RESAMPLE_DIM = '__resample_dim__'
@@ -277,15 +277,14 @@ def reduce(self, func, dim=None, keep_attrs=False, **kwargs):
         """Reduce the items in this group by applying `func` along the
         pre-defined resampling dimension.
 
-        Note that `dim` is by default here and ignored if passed by the user;
-        this ensures compatibility with the existing reduce interface.
-
         Parameters
         ----------
         func : function
             Function which can be called in the form
             `func(x, axis=axis, **kwargs)` to return the result of collapsing
             an np.ndarray over an integer valued axis.
+        dim : str or sequence of str, optional
+            Dimension(s) over which to apply `func`.
         keep_attrs : bool, optional
             If True, the datasets's attributes (`attrs`) will be copied from
             the original object to the new one.  If False (default), the new
@@ -299,8 +298,11 @@ def reduce(self, func, dim=None, keep_attrs=False, **kwargs):
             Array with summarized data and the indicated dimension(s)
             removed.
         """
+        if dim == DEFAULT_DIMS:
+            dim = None
+
         return super(DatasetResample, self).reduce(
-            func, self._dim, keep_attrs, **kwargs)
+            func, dim, keep_attrs, **kwargs)
 
     def _interpolate(self, kind='linear'):
         """Apply scipy.interpolate.interp1d along resampling dimension."""

xarray/core/variable.py

@@ -1258,6 +1258,8 @@ def reduce(self, func, dim=None, axis=None, keep_attrs=False,
             Array with summarized data and the indicated dimension(s)
             removed.
         """
+        if dim == common.ALL_DIMS:
+            dim = None
         if dim is not None and axis is not None:
             raise ValueError("cannot supply both 'axis' and 'dim' arguments")
 

xarray/tests/test_dask.py

@@ -378,8 +378,8 @@ def test_groupby(self):
         u = self.eager_array
         v = self.lazy_array
 
-        expected = u.groupby('x').mean()
-        actual = v.groupby('x').mean()
+        expected = u.groupby('x').mean(xr.ALL_DIMS)
+        actual = v.groupby('x').mean(xr.ALL_DIMS)
         self.assertLazyAndAllClose(expected, actual)
 
     def test_groupby_first(self):

xarray/tests/test_dataarray.py

@@ -14,7 +14,7 @@
     DataArray, Dataset, IndexVariable, Variable, align, broadcast, set_options)
 from xarray.convert import from_cdms2
 from xarray.coding.times import CFDatetimeCoder, _import_cftime
-from xarray.core.common import full_like
+from xarray.core.common import full_like, ALL_DIMS
 from xarray.core.pycompat import OrderedDict, iteritems
 from xarray.tests import (
     ReturnItem, TestCase, assert_allclose, assert_array_equal, assert_equal,
@@ -1983,15 +1983,15 @@ def test_groupby_sum(self):
                                                 self.x[:, 10:].sum(),
                                                 self.x[:, 9:10].sum()]).T),
              'abc': Variable(['abc'], np.array(['a', 'b', 'c']))})['foo']
-        assert_allclose(expected_sum_all, grouped.reduce(np.sum))
-        assert_allclose(expected_sum_all, grouped.sum())
+        assert_allclose(expected_sum_all, grouped.reduce(np.sum, dim=ALL_DIMS))
+        assert_allclose(expected_sum_all, grouped.sum(ALL_DIMS))
 
         expected = DataArray([array['y'].values[idx].sum() for idx
                               in [slice(9), slice(10, None), slice(9, 10)]],
                              [['a', 'b', 'c']], ['abc'])
         actual = array['y'].groupby('abc').apply(np.sum)
         assert_allclose(expected, actual)
-        actual = array['y'].groupby('abc').sum()
+        actual = array['y'].groupby('abc').sum(ALL_DIMS)
         assert_allclose(expected, actual)
 
         expected_sum_axis1 = Dataset(
@@ -2002,6 +2002,24 @@ def test_groupby_sum(self):
         assert_allclose(expected_sum_axis1, grouped.reduce(np.sum, 'y'))
         assert_allclose(expected_sum_axis1, grouped.sum('y'))
 
+    def test_groupby_warning(self):
+        array = self.make_groupby_example_array()
+        grouped = array.groupby('y')
+        with pytest.warns(FutureWarning):
+            grouped.sum()
+
+    def test_groupby_sum_default(self):
+        array = self.make_groupby_example_array()
+        grouped = array.groupby('abc')
+
+        expected_sum_all = Dataset(
+            {'foo': Variable(['x', 'abc'],
+             np.array([self.x[:, :9].sum(axis=-1),
+                       self.x[:, 10:].sum(axis=-1),
+                       self.x[:, 9:10].sum(axis=-1)]).T),
+             'abc': Variable(['abc'], np.array(['a', 'b', 'c']))})['foo']
+        assert_allclose(expected_sum_all, grouped.sum(dim=None))
+
     def test_groupby_count(self):
         array = DataArray(
             [0, 0, np.nan, np.nan, 0, 0],
@@ -2082,9 +2100,9 @@ def test_groupby_math(self):
             assert_identical(expected, actual)
 
         grouped = array.groupby('abc')
-        expected_agg = (grouped.mean() - np.arange(3)).rename(None)
+        expected_agg = (grouped.mean(ALL_DIMS) - np.arange(3)).rename(None)
         actual = grouped - DataArray(range(3), [('abc', ['a', 'b', 'c'])])
-        actual_agg = actual.groupby('abc').mean()
+        actual_agg = actual.groupby('abc').mean(ALL_DIMS)
         assert_allclose(expected_agg, actual_agg)
 
         with raises_regex(TypeError, 'only support binary ops'):
@@ -2158,7 +2176,7 @@ def test_groupby_multidim(self):
                 ('lon', DataArray([5, 28, 23],
                                   coords=[('lon', [30., 40., 50.])])),
                 ('lat', DataArray([16, 40], coords=[('lat', [10., 20.])]))]:
-            actual_sum = array.groupby(dim).sum()
+            actual_sum = array.groupby(dim).sum(ALL_DIMS)
             assert_identical(expected_sum, actual_sum)
 
     def test_groupby_multidim_apply(self):

xarray/tests/test_dataset.py

@@ -14,7 +14,7 @@
 import xarray as xr
 from xarray import (
     DataArray, Dataset, IndexVariable, MergeError, Variable, align, backends,
-    broadcast, open_dataset, set_options)
+    broadcast, open_dataset, set_options, ALL_DIMS)
 from xarray.core import indexing, utils
 from xarray.core.common import full_like
 from xarray.core.pycompat import (
@@ -2609,20 +2609,28 @@ def test_groupby_reduce(self):
 
         expected = data.mean('y')
         expected['yonly'] = expected['yonly'].variable.set_dims({'x': 3})
-        actual = data.groupby('x').mean()
+        actual = data.groupby('x').mean(ALL_DIMS)
         assert_allclose(expected, actual)
 
         actual = data.groupby('x').mean('y')
         assert_allclose(expected, actual)
 
         letters = data['letters']
-        expected = Dataset({'xy': data['xy'].groupby(letters).mean(),
+        expected = Dataset({'xy': data['xy'].groupby(letters).mean(ALL_DIMS),
                             'xonly': (data['xonly'].mean().variable
                                       .set_dims({'letters': 2})),
                             'yonly': data['yonly'].groupby(letters).mean()})
-        actual = data.groupby('letters').mean()
+        actual = data.groupby('letters').mean(ALL_DIMS)
         assert_allclose(expected, actual)
 
+    def test_groupby_warn(self):
+        data = Dataset({'xy': (['x', 'y'], np.random.randn(3, 4)),
+                        'xonly': ('x', np.random.randn(3)),
+                        'yonly': ('y', np.random.randn(4)),
+                        'letters': ('y', ['a', 'a', 'b', 'b'])})
+        with pytest.warns(FutureWarning):
+            data.groupby('x').mean()
+
     def test_groupby_math(self):
         def reorder_dims(x):
             return x.transpose('dim1', 'dim2', 'dim3', 'time')
@@ -2677,7 +2685,7 @@ def test_groupby_math_virtual(self):
         ds = Dataset({'x': ('t', [1, 2, 3])},
                      {'t': pd.date_range('20100101', periods=3)})
         grouped = ds.groupby('t.day')
-        actual = grouped - grouped.mean()
+        actual = grouped - grouped.mean(ALL_DIMS)
         expected = Dataset({'x': ('t', [0, 0, 0])},
                            ds[['t', 't.day']])
         assert_identical(actual, expected)
@@ -2686,18 +2694,17 @@ def test_groupby_nan(self):
         # nan should be excluded from groupby
         ds = Dataset({'foo': ('x', [1, 2, 3, 4])},
                      {'bar': ('x', [1, 1, 2, np.nan])})
-        actual = ds.groupby('bar').mean()
+        actual = ds.groupby('bar').mean(ALL_DIMS)
         expected = Dataset({'foo': ('bar', [1.5, 3]), 'bar': [1, 2]})
         assert_identical(actual, expected)
 
     def test_groupby_order(self):
         # groupby should preserve variables order
-
         ds = Dataset()
         for vn in ['a', 'b', 'c']:
             ds[vn] = DataArray(np.arange(10), dims=['t'])
         data_vars_ref = list(ds.data_vars.keys())
-        ds = ds.groupby('t').mean()
+        ds = ds.groupby('t').mean(ALL_DIMS)
         data_vars = list(ds.data_vars.keys())
         assert data_vars == data_vars_ref
         # coords are now at the end of the list, so the test below fails