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

Deprecation: groupby, resample default dim. #3313

Merged
merged 3 commits into from
Sep 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ Breaking changes
error in a later release.

(:issue:`3250`) by `Guido Imperiale <https://github.com/crusaderky>`_.
- The default dimension for :py:meth:`~xarray.Dataset.groupby`, :py:meth:`~xarray.Dataset.resample`,
:py:meth:`~xarray.DataArray.groupby` and :py:meth:`~xarray.DataArray.resample` reductions is now the
grouping or resampling dimension.
- :py:meth:`~Dataset.to_dataset` requires ``name`` to be passed as a kwarg (previously ambiguous
positional arguments were deprecated)
- Reindexing with variables of a different dimension now raise an error (previously deprecated)
Expand Down
6 changes: 2 additions & 4 deletions xarray/core/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -3875,9 +3875,7 @@ def reduce(
Dataset with this object's DataArrays replaced with new DataArrays
of summarized data and the indicated dimension(s) removed.
"""
if dim is ALL_DIMS:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the two changes in this file are minor unrelated cleanups.

dim = None
if dim is None:
if dim is None or dim is ALL_DIMS:
dims = set(self.dims)
elif isinstance(dim, str) or not isinstance(dim, Iterable):
dims = {dim}
Expand Down Expand Up @@ -4803,7 +4801,7 @@ def quantile(

if isinstance(dim, str):
dims = {dim}
elif dim is None:
elif dim is None or dim is ALL_DIMS:
dims = set(self.dims)
else:
dims = set(dim)
Expand Down
109 changes: 5 additions & 104 deletions xarray/core/groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import numpy as np
import pandas as pd

from . import dtypes, duck_array_ops, nputils, ops, utils
from . import dtypes, duck_array_ops, nputils, ops
from .arithmetic import SupportsArithmetic
from .common import ALL_DIMS, ImplementsArrayReduce, ImplementsDatasetReduce
from .common import ImplementsArrayReduce, ImplementsDatasetReduce
from .concat import concat
from .options import _get_keep_attrs
from .pycompat import integer_types
Expand Down Expand Up @@ -700,19 +700,8 @@ def quantile(self, q, dim=None, interpolation="linear", keep_attrs=None):
numpy.nanpercentile, pandas.Series.quantile, Dataset.quantile,
DataArray.quantile
"""
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 in a future version of xarray. To "
"silence this warning, pass dim=xarray.ALL_DIMS "
"explicitly.",
FutureWarning,
stacklevel=2,
)
if dim is None:
dim = self._group_dim

out = self.apply(
self._obj.__class__.quantile,
Expand Down Expand Up @@ -758,20 +747,6 @@ def reduce(
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 in a future version of xarray. To "
"silence this warning, pass dim=xarray.ALL_DIMS "
"explicitly.",
FutureWarning,
stacklevel=2,
)

if keep_attrs is None:
keep_attrs = _get_keep_attrs(default=False)

Expand All @@ -780,43 +755,6 @@ def reduce_array(ar):

return self.apply(reduce_array, shortcut=shortcut)

# TODO remove the following class method and DEFAULT_DIMS after the
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blindly followed instructions here...

# 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=None,
**kwargs
):
return self.reduce(
func,
dim,
axis,
keep_attrs=keep_attrs,
skipna=skipna,
allow_lazy=True,
**kwargs
)

else:

def wrapped_func( # type: ignore
self, dim=DEFAULT_DIMS, axis=None, keep_attrs=None, **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)
Expand Down Expand Up @@ -898,19 +836,7 @@ def reduce(self, func, dim=None, keep_attrs=None, **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 in a future version of xarray. To "
"silence this warning, pass dim=xarray.ALL_DIMS "
"explicitly.",
FutureWarning,
stacklevel=2,
)
elif dim is None:
if dim is None:
dim = self._group_dim

if keep_attrs is None:
Expand All @@ -921,31 +847,6 @@ def reduce_dataset(ds):

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, skipna=None, **kwargs):
return self.reduce(
func,
dim,
skipna=skipna,
numeric_only=numeric_only,
allow_lazy=True,
**kwargs
)

else:

def wrapped_func(self, dim=DEFAULT_DIMS, **kwargs): # type: ignore
return self.reduce(
func, dim, numeric_only=numeric_only, allow_lazy=True, **kwargs
)

return wrapped_func

def assign(self, **kwargs):
"""Assign data variables by group.

Expand Down
5 changes: 1 addition & 4 deletions xarray/core/resample.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from . import ops
from .groupby import DEFAULT_DIMS, DataArrayGroupBy, DatasetGroupBy
from .groupby import DataArrayGroupBy, DatasetGroupBy

RESAMPLE_DIM = "__resample_dim__"

Expand Down Expand Up @@ -307,9 +307,6 @@ def reduce(self, func, dim=None, keep_attrs=None, **kwargs):
Array with summarized data and the indicated dimension(s)
removed.
"""
if dim == DEFAULT_DIMS:
dim = None

return super().reduce(func, dim, keep_attrs, **kwargs)


Expand Down
12 changes: 1 addition & 11 deletions xarray/tests/test_dataarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -2499,16 +2499,6 @@ 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()

@pytest.mark.skipif(
LooseVersion(xr.__version__) < LooseVersion("0.13"),
reason="not to forget the behavior change",
)
def test_groupby_sum_default(self):
array = self.make_groupby_example_array()
grouped = array.groupby("abc")
Expand All @@ -2529,7 +2519,7 @@ def test_groupby_sum_default(self):
}
)["foo"]

assert_allclose(expected_sum_all, grouped.sum())
assert_allclose(expected_sum_all, grouped.sum(dim="y"))

def test_groupby_count(self):
array = DataArray(
Expand Down
12 changes: 0 additions & 12 deletions xarray/tests/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -3367,18 +3367,6 @@ def test_groupby_reduce(self):
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")
Expand Down
10 changes: 5 additions & 5 deletions xarray/tests/test_groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,21 @@ def test_da_groupby_quantile():
[("x", [1, 1, 1, 2, 2]), ("y", [0, 0, 1])],
)

actual_x = array.groupby("x").quantile(0)
actual_x = array.groupby("x").quantile(0, dim=xr.ALL_DIMS)
expected_x = xr.DataArray([1, 4], [("x", [1, 2])])
assert_identical(expected_x, actual_x)

actual_y = array.groupby("y").quantile(0)
actual_y = array.groupby("y").quantile(0, dim=xr.ALL_DIMS)
expected_y = xr.DataArray([1, 22], [("y", [0, 1])])
assert_identical(expected_y, actual_y)

actual_xx = array.groupby("x").quantile(0, dim="x")
actual_xx = array.groupby("x").quantile(0)
expected_xx = xr.DataArray(
[[1, 11, 22], [4, 15, 24]], [("x", [1, 2]), ("y", [0, 0, 1])]
)
assert_identical(expected_xx, actual_xx)

actual_yy = array.groupby("y").quantile(0, dim="y")
actual_yy = array.groupby("y").quantile(0)
expected_yy = xr.DataArray(
[[1, 26], [2, 22], [3, 23], [4, 24], [5, 25]],
[("x", [1, 1, 1, 2, 2]), ("y", [0, 1])],
Expand All @@ -164,7 +164,7 @@ def test_da_groupby_quantile():
)
g = foo.groupby(foo.time.dt.month)

actual = g.quantile(0)
actual = g.quantile(0, dim=xr.ALL_DIMS)
expected = xr.DataArray(
[
0.0,
Expand Down