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.

Sign up for GitHub

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

Jump to bottom

Deprecation: groupby, resample default dim. #3313

Merged
merged 3 commits into from
Sep 17, 2019
Merged

Deprecation: groupby, resample default dim. #3313

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95635cb
Deprecation: groupby, resample default dim.
dcherian Sep 17, 2019
48eb56a
fix whats-new
dcherian Sep 17, 2019
38deefd
found another test to fix.
dcherian Sep 17, 2019
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ Breaking changes
- Most xarray objects now define ``__slots__``. This reduces overall RAM usage by ~22%
(not counting the underlying numpy buffers); on CPython 3.7/x64, a trivial DataArray
has gone down from 1.9kB to 1.5kB.
- 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.
dcherian marked this conversation as resolved.
Show resolved Hide resolved

Caveats:

Expand Down
6 changes: 2 additions & 4 deletions xarray/core/dataset.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
6 changes: 0 additions & 6 deletions xarray/tests/test_dataarray.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2499,12 +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",
Expand Down
12 changes: 0 additions & 12 deletions xarray/tests/test_dataset.py
View file
Open in desktop
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
View file
Open in desktop
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