From 1ec754f31d0a0654f3f8d6a3a9513590a187399f Mon Sep 17 00:00:00 2001 From: MBrouns Date: Sat, 20 Jun 2020 12:50:32 +0200 Subject: [PATCH 1/4] Deprecate `center` on `df.expanding` `df.expanding(center=True)` currently returns non-sensical results and it is unclear what results would be expected. It was previously removed in https://github.com/pandas-dev/pandas/pull/7934 for that same reason, but was reintroduced for unclear reasons in a later refactoring --- doc/source/whatsnew/v1.1.0.rst | 3 +++ pandas/core/base.py | 18 +++++++++++------ pandas/core/generic.py | 2 +- pandas/core/groupby/base.py | 9 +++++++-- pandas/core/window/expanding.py | 18 +++++++++++++++-- .../test_moments_consistency_expanding.py | 10 +++++----- pandas/tests/window/test_api.py | 11 ++-------- pandas/tests/window/test_expanding.py | 20 ++++++++++++------- 8 files changed, 59 insertions(+), 32 deletions(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 9bd4ddbb624d9..e7f417cbe52c3 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -820,6 +820,9 @@ Deprecations precision through the ``rtol``, and ``atol`` parameters, thus deprecating the ``check_less_precise`` parameter. (:issue:`13357`). - :func:`DataFrame.melt` accepting a value_name that already exists is deprecated, and will be removed in a future version (:issue:`34731`) +- the ``center`` keyword in the :meth:`DataFrame.expanding` function is deprecated and will be removed in a future version (:issue:`20647`) + + .. --------------------------------------------------------------------------- diff --git a/pandas/core/base.py b/pandas/core/base.py index b62ef668df5e1..2297f8337ca0b 100644 --- a/pandas/core/base.py +++ b/pandas/core/base.py @@ -5,6 +5,7 @@ import builtins import textwrap from typing import Any, Dict, FrozenSet, List, Optional, Union +import warnings import numpy as np @@ -581,12 +582,17 @@ def _shallow_copy(self, obj, **kwargs): """ return a new object with the replacement attributes """ - if isinstance(obj, self._constructor): - obj = obj.obj - for attr in self._attributes: - if attr not in kwargs: - kwargs[attr] = getattr(self, attr) - return self._constructor(obj, **kwargs) + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", + "The `center` argument on `expanding` will be removed in the future", + ) + if isinstance(obj, self._constructor): + obj = obj.obj + for attr in self._attributes: + if attr not in kwargs: + kwargs[attr] = getattr(self, attr) + return self._constructor(obj, **kwargs) class IndexOpsMixin: diff --git a/pandas/core/generic.py b/pandas/core/generic.py index d892e2487b31c..7e7a5ae92e2a2 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -10501,7 +10501,7 @@ def rolling( cls.rolling = rolling @doc(Expanding) - def expanding(self, min_periods=1, center=False, axis=0): + def expanding(self, min_periods=1, center=None, axis=0): axis = self._get_axis_number(axis) return Expanding(self, min_periods=min_periods, center=center, axis=axis) diff --git a/pandas/core/groupby/base.py b/pandas/core/groupby/base.py index e71b2f94c8014..7dded2ba258e3 100644 --- a/pandas/core/groupby/base.py +++ b/pandas/core/groupby/base.py @@ -4,6 +4,7 @@ SeriesGroupBy and the DataFrameGroupBy objects. """ import collections +import warnings from pandas.core.dtypes.common import is_list_like, is_scalar @@ -40,8 +41,12 @@ def _gotitem(self, key, ndim, subset=None): groupby = self._groupby[key] except IndexError: groupby = self._groupby - - self = type(self)(subset, groupby=groupby, parent=self, **kwargs) + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", + "The `center` argument on `expanding` will be removed in the future", + ) + self = type(self)(subset, groupby=groupby, parent=self, **kwargs) self._reset_cache() if subset.ndim == 2: if is_scalar(key) and key in subset or is_list_like(key): diff --git a/pandas/core/window/expanding.py b/pandas/core/window/expanding.py index bbc19fad8b799..6fcb3b5dcbc2b 100644 --- a/pandas/core/window/expanding.py +++ b/pandas/core/window/expanding.py @@ -1,5 +1,6 @@ from textwrap import dedent from typing import Dict, Optional +import warnings from pandas.compat.numpy import function as nv from pandas.util._decorators import Appender, Substitution, doc @@ -57,7 +58,15 @@ class Expanding(_Rolling_and_Expanding): _attributes = ["min_periods", "center", "axis"] - def __init__(self, obj, min_periods=1, center=False, axis=0, **kwargs): + def __init__(self, obj, min_periods=1, center=None, axis=0, **kwargs): + if center is not None: + warnings.warn( + "The `center` argument on `expanding` will be removed in the future", + FutureWarning, + stacklevel=3, + ) + else: + center = False super().__init__(obj=obj, min_periods=min_periods, center=center, axis=axis) @property @@ -129,7 +138,12 @@ def aggregate(self, func, *args, **kwargs): @Substitution(name="expanding") @Appender(_shared_docs["count"]) def count(self, **kwargs): - return super().count(**kwargs) + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", + "The `center` argument on `expanding` will be removed in the future", + ) + return super().count(**kwargs) @Substitution(name="expanding") @Appender(_shared_docs["apply"]) diff --git a/pandas/tests/window/moments/test_moments_consistency_expanding.py b/pandas/tests/window/moments/test_moments_consistency_expanding.py index ee3579d76d1db..3ec91dcb60610 100644 --- a/pandas/tests/window/moments/test_moments_consistency_expanding.py +++ b/pandas/tests/window/moments/test_moments_consistency_expanding.py @@ -119,8 +119,8 @@ def test_expanding_corr_pairwise(frame): ids=["sum", "mean", "max", "min"], ) def test_expanding_func(func, static_comp, has_min_periods, series, frame, nan_locs): - def expanding_func(x, min_periods=1, center=False, axis=0): - exp = x.expanding(min_periods=min_periods, center=center, axis=axis) + def expanding_func(x, min_periods=1, axis=0): + exp = x.expanding(min_periods=min_periods, axis=axis) return getattr(exp, func)() _check_expanding( @@ -166,7 +166,7 @@ def test_expanding_apply_consistency( with warnings.catch_warnings(): warnings.filterwarnings( - "ignore", message=".*(empty slice|0 for slice).*", category=RuntimeWarning, + "ignore", message=".*(empty slice|0 for slice).*", category=RuntimeWarning ) # test consistency between expanding_xyz() and either (a) # expanding_apply of Series.xyz(), or (b) expanding_apply of @@ -267,7 +267,7 @@ def test_expanding_consistency(consistency_data, min_periods): # with empty/0-length Series/DataFrames with warnings.catch_warnings(): warnings.filterwarnings( - "ignore", message=".*(empty slice|0 for slice).*", category=RuntimeWarning, + "ignore", message=".*(empty slice|0 for slice).*", category=RuntimeWarning ) # test consistency between different expanding_* moments @@ -454,7 +454,7 @@ def test_expanding_cov_pairwise_diff_length(): def test_expanding_corr_pairwise_diff_length(): # GH 7512 df1 = DataFrame( - [[1, 2], [3, 2], [3, 4]], columns=["A", "B"], index=Index(range(3), name="bar"), + [[1, 2], [3, 2], [3, 4]], columns=["A", "B"], index=Index(range(3), name="bar") ) df1a = DataFrame( [[1, 2], [3, 4]], index=Index([0, 2], name="bar"), columns=["A", "B"] diff --git a/pandas/tests/window/test_api.py b/pandas/tests/window/test_api.py index 33fb79d98a324..2c3d8b4608806 100644 --- a/pandas/tests/window/test_api.py +++ b/pandas/tests/window/test_api.py @@ -107,10 +107,7 @@ def test_agg(): with pytest.raises(SpecificationError, match=msg): r.aggregate( - { - "A": {"mean": "mean", "sum": "sum"}, - "B": {"mean2": "mean", "sum2": "sum"}, - } + {"A": {"mean": "mean", "sum": "sum"}, "B": {"mean2": "mean", "sum2": "sum"}} ) result = r.aggregate({"A": ["mean", "std"], "B": ["mean", "std"]}) @@ -191,11 +188,7 @@ def test_count_nonnumeric_types(): "dt_nat", "periods_nat", ] - dt_nat_col = [ - Timestamp("20170101"), - Timestamp("20170203"), - Timestamp(None), - ] + dt_nat_col = [Timestamp("20170101"), Timestamp("20170203"), Timestamp(None)] df = DataFrame( { diff --git a/pandas/tests/window/test_expanding.py b/pandas/tests/window/test_expanding.py index 30d65ebe84a1f..1c53d72ec719f 100644 --- a/pandas/tests/window/test_expanding.py +++ b/pandas/tests/window/test_expanding.py @@ -18,13 +18,10 @@ def test_doc_string(): def test_constructor(which): # GH 12669 - c = which.expanding # valid c(min_periods=1) - c(min_periods=1, center=True) - c(min_periods=1, center=False) # not valid for w in [2.0, "foo", np.array([2])]: @@ -32,10 +29,6 @@ def test_constructor(which): with pytest.raises(ValueError, match=msg): c(min_periods=w) - msg = "center must be a boolean" - with pytest.raises(ValueError, match=msg): - c(min_periods=1, center=w) - @pytest.mark.parametrize("method", ["std", "mean", "sum", "max", "min", "var"]) def test_numpy_compat(method): @@ -213,3 +206,16 @@ def test_iter_expanding_series(ser, expected, min_periods): for (expected, actual) in zip(expected, ser.expanding(min_periods)): tm.assert_series_equal(actual, expected) + + +def test_center_deprecate_warning(): + # GH 20647 + df = pd.DataFrame() + with tm.assert_produces_warning(FutureWarning): + df.expanding(center=True) + + with tm.assert_produces_warning(FutureWarning): + df.expanding(center=False) + + with tm.assert_produces_warning(None): + df.expanding() From c1da8160e7ecd204db5ed5b14139ee78a73d8b5e Mon Sep 17 00:00:00 2001 From: MBrouns Date: Tue, 7 Jul 2020 09:31:12 +0200 Subject: [PATCH 2/4] remove filterwarnings from core copying behaviour --- pandas/core/base.py | 18 ++++++------------ pandas/core/generic.py | 10 ++++++++++ pandas/core/groupby/base.py | 9 ++------- pandas/core/window/expanding.py | 8 -------- 4 files changed, 18 insertions(+), 27 deletions(-) diff --git a/pandas/core/base.py b/pandas/core/base.py index 2297f8337ca0b..b62ef668df5e1 100644 --- a/pandas/core/base.py +++ b/pandas/core/base.py @@ -5,7 +5,6 @@ import builtins import textwrap from typing import Any, Dict, FrozenSet, List, Optional, Union -import warnings import numpy as np @@ -582,17 +581,12 @@ def _shallow_copy(self, obj, **kwargs): """ return a new object with the replacement attributes """ - with warnings.catch_warnings(): - warnings.filterwarnings( - "ignore", - "The `center` argument on `expanding` will be removed in the future", - ) - if isinstance(obj, self._constructor): - obj = obj.obj - for attr in self._attributes: - if attr not in kwargs: - kwargs[attr] = getattr(self, attr) - return self._constructor(obj, **kwargs) + if isinstance(obj, self._constructor): + obj = obj.obj + for attr in self._attributes: + if attr not in kwargs: + kwargs[attr] = getattr(self, attr) + return self._constructor(obj, **kwargs) class IndexOpsMixin: diff --git a/pandas/core/generic.py b/pandas/core/generic.py index 7e7a5ae92e2a2..ff4632efaeebb 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -10503,6 +10503,16 @@ def rolling( @doc(Expanding) def expanding(self, min_periods=1, center=None, axis=0): axis = self._get_axis_number(axis) + if center is not None: + warnings.warn( + "The `center` argument on `expanding` " + "will be removed in the future", + FutureWarning, + stacklevel=3, + ) + else: + center = False + return Expanding(self, min_periods=min_periods, center=center, axis=axis) cls.expanding = expanding diff --git a/pandas/core/groupby/base.py b/pandas/core/groupby/base.py index 7dded2ba258e3..e71b2f94c8014 100644 --- a/pandas/core/groupby/base.py +++ b/pandas/core/groupby/base.py @@ -4,7 +4,6 @@ SeriesGroupBy and the DataFrameGroupBy objects. """ import collections -import warnings from pandas.core.dtypes.common import is_list_like, is_scalar @@ -41,12 +40,8 @@ def _gotitem(self, key, ndim, subset=None): groupby = self._groupby[key] except IndexError: groupby = self._groupby - with warnings.catch_warnings(): - warnings.filterwarnings( - "ignore", - "The `center` argument on `expanding` will be removed in the future", - ) - self = type(self)(subset, groupby=groupby, parent=self, **kwargs) + + self = type(self)(subset, groupby=groupby, parent=self, **kwargs) self._reset_cache() if subset.ndim == 2: if is_scalar(key) and key in subset or is_list_like(key): diff --git a/pandas/core/window/expanding.py b/pandas/core/window/expanding.py index 6fcb3b5dcbc2b..af04d24e0fb44 100644 --- a/pandas/core/window/expanding.py +++ b/pandas/core/window/expanding.py @@ -59,14 +59,6 @@ class Expanding(_Rolling_and_Expanding): _attributes = ["min_periods", "center", "axis"] def __init__(self, obj, min_periods=1, center=None, axis=0, **kwargs): - if center is not None: - warnings.warn( - "The `center` argument on `expanding` will be removed in the future", - FutureWarning, - stacklevel=3, - ) - else: - center = False super().__init__(obj=obj, min_periods=min_periods, center=center, axis=axis) @property From 31356f2bc30c6263bcce5e09079c648ef1a29f4f Mon Sep 17 00:00:00 2001 From: MBrouns Date: Tue, 7 Jul 2020 14:30:29 +0200 Subject: [PATCH 3/4] correct stacklevel on deprecationwarning --- pandas/core/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/generic.py b/pandas/core/generic.py index ff4632efaeebb..fa15385fed930 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -10508,7 +10508,7 @@ def expanding(self, min_periods=1, center=None, axis=0): "The `center` argument on `expanding` " "will be removed in the future", FutureWarning, - stacklevel=3, + stacklevel=2, ) else: center = False From 33f3fb7a4e292e1044f4a02a513125fe9cdf0c5d Mon Sep 17 00:00:00 2001 From: MBrouns Date: Tue, 7 Jul 2020 15:42:42 +0200 Subject: [PATCH 4/4] revert changes to constructor test + remove filterwarning in count as that shouldn't throw warnings anymore --- pandas/core/window/expanding.py | 8 +------- pandas/tests/window/test_expanding.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pandas/core/window/expanding.py b/pandas/core/window/expanding.py index af04d24e0fb44..8267cd4f0971e 100644 --- a/pandas/core/window/expanding.py +++ b/pandas/core/window/expanding.py @@ -1,6 +1,5 @@ from textwrap import dedent from typing import Dict, Optional -import warnings from pandas.compat.numpy import function as nv from pandas.util._decorators import Appender, Substitution, doc @@ -130,12 +129,7 @@ def aggregate(self, func, *args, **kwargs): @Substitution(name="expanding") @Appender(_shared_docs["count"]) def count(self, **kwargs): - with warnings.catch_warnings(): - warnings.filterwarnings( - "ignore", - "The `center` argument on `expanding` will be removed in the future", - ) - return super().count(**kwargs) + return super().count(**kwargs) @Substitution(name="expanding") @Appender(_shared_docs["apply"]) diff --git a/pandas/tests/window/test_expanding.py b/pandas/tests/window/test_expanding.py index 1c53d72ec719f..146eca07c523e 100644 --- a/pandas/tests/window/test_expanding.py +++ b/pandas/tests/window/test_expanding.py @@ -16,12 +16,18 @@ def test_doc_string(): df.expanding(2).sum() +@pytest.mark.filterwarnings( + "ignore:The `center` argument on `expanding` will be removed in the future" +) def test_constructor(which): # GH 12669 + c = which.expanding # valid c(min_periods=1) + c(min_periods=1, center=True) + c(min_periods=1, center=False) # not valid for w in [2.0, "foo", np.array([2])]: @@ -29,6 +35,10 @@ def test_constructor(which): with pytest.raises(ValueError, match=msg): c(min_periods=w) + msg = "center must be a boolean" + with pytest.raises(ValueError, match=msg): + c(min_periods=1, center=w) + @pytest.mark.parametrize("method", ["std", "mean", "sum", "max", "min", "var"]) def test_numpy_compat(method):