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

DEPR: Change raw kwarg in rolling/expanding.apply to False #29829

Merged
merged 3 commits into from
Nov 26, 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
2 changes: 2 additions & 0 deletions doc/source/whatsnew/v1.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,8 @@ or ``matplotlib.Axes.plot``. See :ref:`plotting.formatters` for more.
- :func:`read_stata` and :meth:`DataFrame.to_stata` no longer supports the "encoding" argument (:issue:`21400`)
- Removed previously deprecated "raise_conflict" argument from :meth:`DataFrame.update`, use "errors" instead (:issue:`23585`)
- Removed previously deprecated keyword "n" from :meth:`DatetimeIndex.shift`, :meth:`TimedeltaIndex.shift`, :meth:`PeriodIndex.shift`, use "periods" instead (:issue:`22458`)
- Changed the default value for the `raw` argument in :func:`Series.rolling().apply() <pandas.core.window.Rolling.apply>`, :func:`DataFrame.rolling().apply() <pandas.core.window.Rolling.apply>`,
- :func:`Series.expanding().apply() <pandas.core.window.Expanding.apply>`, and :func:`DataFrame.expanding().apply() <pandas.core.window.Expanding.apply>` to ``False`` (:issue:`20584`)
-

.. _whatsnew_1000.performance:
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/window/expanding.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def count(self, **kwargs):

@Substitution(name="expanding")
@Appender(_shared_docs["apply"])
def apply(self, func, raw=None, args=(), kwargs={}):
def apply(self, func, raw=False, args=(), kwargs={}):
return super().apply(func, raw=raw, args=args, kwargs=kwargs)

@Substitution(name="expanding")
Expand Down
27 changes: 5 additions & 22 deletions pandas/core/window/rolling.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from functools import partial
from textwrap import dedent
from typing import Callable, Dict, List, Optional, Set, Tuple, Union
import warnings

import numpy as np

Expand Down Expand Up @@ -1190,15 +1189,11 @@ def count(self):
raw : bool, default None
* ``False`` : passes each row or column as a Series to the
function.
* ``True`` or ``None`` : the passed function will receive ndarray
* ``True`` : the passed function will receive ndarray
objects instead.
If you are just applying a NumPy reduction function this will
achieve much better performance.
The `raw` parameter is required and will show a FutureWarning if
not passed. In the future `raw` will default to False.
.. versionadded:: 0.23.0
*args, **kwargs
Arguments and keyword arguments to be passed into func.
Expand All @@ -1214,27 +1209,15 @@ def count(self):
"""
)

def apply(self, func, raw=None, args=(), kwargs={}):
def apply(self, func, raw=False, args=(), kwargs={}):
Copy link
Member

Choose a reason for hiding this comment

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

can this now be annotated?

Doesnt have to be for this PR, but kwargs should be changed to not have a mutable default

Copy link
Member Author

Choose a reason for hiding this comment

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

I can do both in a follow up.

Agreed that this should be kwargs=None. The docstring is misleading as well since it says apply takes **kwargs so reason enough for a cleanup + full change here.

from pandas import Series

kwargs.pop("_level", None)
kwargs.pop("floor", None)
window = self._get_window()
offset = _offset(window, self.center)

# TODO: default is for backward compat
# change to False in the future
if raw is None:
warnings.warn(
"Currently, 'apply' passes the values as ndarrays to the "
"applied function. In the future, this will change to passing "
"it as Series objects. You need to specify 'raw=True' to keep "
"the current behaviour, and you can pass 'raw=False' to "
"silence this warning",
FutureWarning,
stacklevel=3,
)
raw = True
if not is_bool(raw):
raise ValueError("raw parameter must be `True` or `False`")

window_func = partial(
self._get_cython_func_type("roll_generic"),
Expand Down Expand Up @@ -1898,7 +1881,7 @@ def count(self):

@Substitution(name="rolling")
@Appender(_shared_docs["apply"])
def apply(self, func, raw=None, args=(), kwargs={}):
def apply(self, func, raw=False, args=(), kwargs={}):
return super().apply(func, raw=raw, args=args, kwargs=kwargs)

@Substitution(name="rolling")
Expand Down
15 changes: 4 additions & 11 deletions pandas/tests/window/test_moments.py
Original file line number Diff line number Diff line change
Expand Up @@ -687,17 +687,10 @@ def f(x):
result = s.rolling(2, min_periods=0).apply(len, raw=raw)
tm.assert_series_equal(result, expected)

@pytest.mark.parametrize("klass", [Series, DataFrame])
@pytest.mark.parametrize(
"method", [lambda x: x.rolling(window=2), lambda x: x.expanding()]
)
def test_apply_future_warning(self, klass, method):

# gh-5071
s = klass(np.arange(3))

with tm.assert_produces_warning(FutureWarning):
method(s).apply(lambda x: len(x))
@pytest.mark.parametrize("bad_raw", [None, 1, 0])
def test_rolling_apply_invalid_raw(self, bad_raw):
with pytest.raises(ValueError, match="raw parameter must be `True` or `False`"):
Series(range(3)).rolling(1).apply(len, raw=bad_raw)

def test_rolling_apply_out_of_bounds(self, raw):
# gh-1850
Expand Down