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: numeric_only default in resampler ops #47177

Merged
merged 3 commits into from
Jun 5, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 6 additions & 1 deletion doc/source/whatsnew/v1.5.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ In the case where ``df.columns`` is not unique, use :meth:`DataFrame.isetitem`:
``numeric_only`` default value
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Across the DataFrame and DataFrameGroupBy operations such as
Across the :class:`DataFrame`, :class:`.DataFrameGroupBy`, and :class:`.Resampler` operations such as
``min``, ``sum``, and ``idxmax``, the default
value of the ``numeric_only`` argument, if it exists at all, was inconsistent.
Furthermore, operations with the default value ``None`` can lead to surprising
Expand Down Expand Up @@ -644,6 +644,11 @@ gained the ``numeric_only`` argument.
- :meth:`.GroupBy.std`
- :meth:`.GroupBy.sem`
- :meth:`.DataFrameGroupBy.quantile`
- :meth:`.Resampler.mean`
- :meth:`.Resampler.median`
- :meth:`.Resampler.sem`
- :meth:`.Resampler.std`
- :meth:`.Resampler.var`

.. _whatsnew_150.deprecations.other:

Expand Down
18 changes: 0 additions & 18 deletions pandas/compat/numpy/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,24 +371,6 @@ def validate_groupby_func(name, args, kwargs, allowed=None) -> None:
)


RESAMPLER_NUMPY_OPS = ("min", "max", "sum", "prod", "mean", "std", "var")


def validate_resampler_func(method: str, args, kwargs) -> None:
"""
'args' and 'kwargs' should be empty because all of their necessary
parameters are explicitly listed in the function signature
"""
if len(args) + len(kwargs) > 0:
if method in RESAMPLER_NUMPY_OPS:
raise UnsupportedFunctionCall(
"numpy operations are not valid with resample. "
f"Use .resample(...).{method}() instead"
)
else:
raise TypeError("too many arguments passed in")


def validate_minmax_axis(axis: int | None, ndim: int = 1) -> None:
"""
Ensure that the axis argument passed to min, max, argmin, or argmax is zero
Expand Down
125 changes: 80 additions & 45 deletions pandas/core/resample.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
TimestampConvertibleTypes,
npt,
)
from pandas.compat.numpy import function as nv
from pandas.errors import (
AbstractMethodError,
DataError,
Expand Down Expand Up @@ -393,7 +392,7 @@ def transform(self, arg, *args, **kwargs):
"""
return self._selected_obj.groupby(self.groupby).transform(arg, *args, **kwargs)

def _downsample(self, f):
def _downsample(self, f, **kwargs):
raise AbstractMethodError(self)

def _upsample(self, f, limit=None, fill_value=None):
Expand Down Expand Up @@ -937,25 +936,27 @@ def asfreq(self, fill_value=None):
"""
return self._upsample("asfreq", fill_value=fill_value)

def std(self, ddof=1, *args, **kwargs):
def std(self, ddof=1, numeric_only: bool = False):
Copy link
Member Author

@rhshadrach rhshadrach Jun 4, 2022

Choose a reason for hiding this comment

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

@mroeschke - I believe you've done a bunch of work in resample/window; do you know why args/kwargs exist on these ops? They seem to be on all these methods only to check that the user didn't pass anything to them. I'm going to revert these removals here, but was thinking about doing a cleanup of these in a separate PR if they aren't useful. Checking the history here, there is some indication of compatibility with numpy in #12810, but I don't understand it.

Copy link
Member

Choose a reason for hiding this comment

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

I think the numpy compatibility was such that one could call e.g. np.mean(df.groupby/resample/rolling) and get the equivalent call to df.groupby/resample/rolling.mean(). Not sure why that compatibility was desired or if it was widely tested (guessing not if tests are passing).

FWIW I think I broke this compatibility too when I added engine keywords to the window and groupby reduction ops, so I'd be +1 to clean these *args, **kwargs

"""
Compute standard deviation of groups, excluding missing values.

Parameters
----------
ddof : int, default 1
Degrees of freedom.
numeric_only : bool, default False
Include only `float`, `int` or `boolean` data.

.. versionadded:: 1.5.0

Returns
-------
DataFrame or Series
Standard deviation of values within each group.
"""
nv.validate_resampler_func("std", args, kwargs)
# error: Unexpected keyword argument "ddof" for "_downsample"
return self._downsample("std", ddof=ddof) # type: ignore[call-arg]
return self._downsample("std", ddof=ddof, numeric_only=numeric_only)

def var(self, ddof=1, *args, **kwargs):
def var(self, ddof=1, numeric_only: bool = False):
"""
Compute variance of groups, excluding missing values.

Expand All @@ -964,14 +965,17 @@ def var(self, ddof=1, *args, **kwargs):
ddof : int, default 1
Degrees of freedom.

numeric_only : bool, default False
Include only `float`, `int` or `boolean` data.

.. versionadded:: 1.5.0

Returns
-------
DataFrame or Series
Variance of values within each group.
"""
nv.validate_resampler_func("var", args, kwargs)
# error: Unexpected keyword argument "ddof" for "_downsample"
return self._downsample("var", ddof=ddof) # type: ignore[call-arg]
return self._downsample("var", ddof=ddof, numeric_only=numeric_only)

@doc(GroupBy.size)
def size(self):
Expand Down Expand Up @@ -1027,53 +1031,84 @@ def quantile(self, q=0.5, **kwargs):
Return a DataFrame, where the coulmns are groupby columns,
and the values are its quantiles.
"""
# error: Unexpected keyword argument "q" for "_downsample"
# error: Too many arguments for "_downsample"
return self._downsample("quantile", q=q, **kwargs) # type: ignore[call-arg]
return self._downsample("quantile", q=q, **kwargs)


# downsample methods
for method in ["sum", "prod", "min", "max", "first", "last"]:
def _add_downsample_kernel(
name: str, args: tuple[str, ...], docs_class: type = GroupBy
) -> None:
"""
Add a kernel to Resampler.

Arguments
---------
name : str
Name of the kernel.
args : tuple
Arguments of the method.
docs_class : type
Class to get kernel docstring from.
"""
assert args in (
("numeric_only", "min_count"),
("numeric_only",),
("ddof", "numeric_only"),
(),
)

def f(
self,
_method: str = method,
numeric_only: bool | lib.NoDefault = lib.no_default,
min_count: int = 0,
*args,
**kwargs,
):
if numeric_only is lib.no_default:
if _method != "sum":
# Explicitly provide args rather than args/kwargs for API docs
if args == ("numeric_only", "min_count"):

def f(
self,
numeric_only: bool | lib.NoDefault = lib.no_default,
min_count: int = 0,
):
if numeric_only is lib.no_default and name != "sum":
# For DataFrameGroupBy, set it to be False for methods other than `sum`.
numeric_only = False

nv.validate_resampler_func(_method, args, kwargs)
return self._downsample(_method, numeric_only=numeric_only, min_count=min_count)

f.__doc__ = getattr(GroupBy, method).__doc__
setattr(Resampler, method, f)

return self._downsample(
name, numeric_only=numeric_only, min_count=min_count
)

# downsample methods
for method in ["mean", "sem", "median", "ohlc"]:
elif args == ("numeric_only",):
# error: All conditional function variants must have identical signatures
def f( # type: ignore[misc]
self, numeric_only: bool | lib.NoDefault = lib.no_default
):
return self._downsample(name, numeric_only=numeric_only)

elif args == ("ddof", "numeric_only"):
# error: All conditional function variants must have identical signatures
def f( # type: ignore[misc]
self,
ddof: int = 1,
numeric_only: bool | lib.NoDefault = lib.no_default,
):
return self._downsample(name, ddof=ddof, numeric_only=numeric_only)

def g(self, _method=method, *args, **kwargs):
nv.validate_resampler_func(_method, args, kwargs)
return self._downsample(_method)
else:
# error: All conditional function variants must have identical signatures
def f( # type: ignore[misc]
self,
):
return self._downsample(name)

g.__doc__ = getattr(GroupBy, method).__doc__
setattr(Resampler, method, g)
f.__doc__ = getattr(docs_class, name).__doc__
setattr(Resampler, name, f)


# series only methods
for method in ["sum", "prod", "min", "max", "first", "last"]:
_add_downsample_kernel(method, ("numeric_only", "min_count"))
for method in ["mean", "median"]:
_add_downsample_kernel(method, ("numeric_only",))
for method in ["sem"]:
_add_downsample_kernel(method, ("ddof", "numeric_only"))
for method in ["ohlc"]:
_add_downsample_kernel(method, ())
for method in ["nunique"]:

def h(self, _method=method):
return self._downsample(_method)

h.__doc__ = getattr(SeriesGroupBy, method).__doc__
setattr(Resampler, method, h)
_add_downsample_kernel(method, (), SeriesGroupBy)


class _GroupByMixin(PandasObject):
Expand Down
15 changes: 0 additions & 15 deletions pandas/tests/resample/test_datetime_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

from pandas._libs import lib
from pandas._typing import DatetimeNaTType
from pandas.errors import UnsupportedFunctionCall

import pandas as pd
from pandas import (
Expand Down Expand Up @@ -226,20 +225,6 @@ def _ohlc(group):
tm.assert_frame_equal(result, expected)


@pytest.mark.parametrize("func", ["min", "max", "sum", "prod", "mean", "var", "std"])
def test_numpy_compat(func):
# see gh-12811
s = Series([1, 2, 3, 4, 5], index=date_range("20130101", periods=5, freq="s"))
r = s.resample("2s")

msg = "numpy operations are not valid with resample"

with pytest.raises(UnsupportedFunctionCall, match=msg):
getattr(r, func)(func, 1, 2, 3)
with pytest.raises(UnsupportedFunctionCall, match=msg):
getattr(r, func)(axis=1)


def test_resample_how_callables():
# GH#7929
data = np.arange(5, dtype=np.int64)
Expand Down
44 changes: 36 additions & 8 deletions pandas/tests/resample/test_resample_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,7 @@ def test_end_and_end_day_origin(


@pytest.mark.parametrize(
# expected_data is a string when op raises a ValueError
"method, numeric_only, expected_data",
[
("sum", True, {"num": [25]}),
Expand All @@ -834,6 +835,21 @@ def test_end_and_end_day_origin(
("last", True, {"num": [20]}),
("last", False, {"cat": ["cat_2"], "num": [20]}),
("last", lib.no_default, {"cat": ["cat_2"], "num": [20]}),
("mean", True, {"num": [12.5]}),
("mean", False, {"num": [12.5]}),
("mean", lib.no_default, {"num": [12.5]}),
("median", True, {"num": [12.5]}),
("median", False, {"num": [12.5]}),
("median", lib.no_default, {"num": [12.5]}),
("std", True, {"num": [10.606601717798213]}),
("std", False, "could not convert string to float"),
("std", lib.no_default, {"num": [10.606601717798213]}),
("var", True, {"num": [112.5]}),
("var", False, "could not convert string to float"),
("var", lib.no_default, {"num": [112.5]}),
("sem", True, {"num": [7.5]}),
("sem", False, "could not convert string to float"),
("sem", lib.no_default, {"num": [7.5]}),
],
)
def test_frame_downsample_method(method, numeric_only, expected_data):
Expand All @@ -845,20 +861,32 @@ def test_frame_downsample_method(method, numeric_only, expected_data):
resampled = df.resample("Y")

func = getattr(resampled, method)
if method == "prod" and numeric_only is not True:
if numeric_only is lib.no_default and method not in (
"min",
"max",
"first",
"last",
"prod",
):
warn = FutureWarning
msg = "Dropping invalid columns in DataFrameGroupBy.prod is deprecated"
elif method == "sum" and numeric_only is lib.no_default:
msg = (
f"default value of numeric_only in DataFrameGroupBy.{method} is deprecated"
)
elif method in ("prod", "mean", "median") and numeric_only is not True:
warn = FutureWarning
msg = "The default value of numeric_only in DataFrameGroupBy.sum is deprecated"
msg = f"Dropping invalid columns in DataFrameGroupBy.{method} is deprecated"
else:
warn = None
msg = ""
with tm.assert_produces_warning(warn, match=msg):
result = func(numeric_only=numeric_only)

expected = DataFrame(expected_data, index=expected_index)
tm.assert_frame_equal(result, expected)
if isinstance(expected_data, str):
klass = TypeError if method == "var" else ValueError
with pytest.raises(klass, match=expected_data):
_ = func(numeric_only=numeric_only)
else:
result = func(numeric_only=numeric_only)
expected = DataFrame(expected_data, index=expected_index)
tm.assert_frame_equal(result, expected)


@pytest.mark.parametrize(
Expand Down