Skip to content

ENH: support datetime64, datetime64tz in nanops.mean, nanops.median #29941

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

Merged
merged 14 commits into from
Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
17 changes: 16 additions & 1 deletion pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -7575,6 +7575,19 @@ def _count_level(self, level, axis=0, numeric_only=False):
def _reduce(
self, op, name, axis=0, skipna=True, numeric_only=None, filter_type=None, **kwds
):

dtype_is_dt = self.dtypes.apply(lambda x: x.kind == "M")
if numeric_only is None and name in ["mean", "median"] and dtype_is_dt.any():
warnings.warn(
"DataFrame.mean and DataFrame.median with numeric_only=None "
"will include datetime64 and datetime64tz columns in a "
"future version.",
FutureWarning,
stacklevel=3,
)
cols = self.columns[~dtype_is_dt]
self = self[cols]

if axis is None and filter_type == "bool":
labels = None
constructor = None
Expand Down Expand Up @@ -7614,8 +7627,10 @@ def _get_data(axis_matters):
# TODO: combine with hasattr(result, 'dtype') further down
# hard since we don't have `values` down there.
result = np.bool_(result)
except TypeError:
except (TypeError, ValueError):
# e.g. in nanops trying to convert strs to float
# TODO: the ValueError is raised in trying to convert str
Copy link
Contributor

Choose a reason for hiding this comment

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

yes this should be a TypeError, where is the ValueError coming from?

Copy link
Contributor

Choose a reason for hiding this comment

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

what is trying to convert to str?

Copy link
Contributor

Choose a reason for hiding this comment

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

do you still need this ValueError? (as i think you changed below)

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'll re-run without catching ValueError. IIRC it was in nanops._ensure_numeric

Copy link
Member Author

Choose a reason for hiding this comment

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

yah, there are 3 tests where x.astype(np.float64) raises a ValueError in _ensure_numeric with a object-dtype x that contains a string.

Copy link
Member Author

Choose a reason for hiding this comment

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

just removed this and re-raised as TypeError on nanops 1292-1296

# to float, should we make that a TypError?

# try by-column first
if filter_type is None and axis == 0:
Expand Down
11 changes: 6 additions & 5 deletions pandas/core/nanops.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
is_timedelta64_dtype,
pandas_dtype,
)
from pandas.core.dtypes.dtypes import DatetimeTZDtype
from pandas.core.dtypes.missing import isna, na_value_for_dtype, notna

bn = import_optional_dependency("bottleneck", raise_on_missing=False, on_version="warn")
Expand Down Expand Up @@ -494,7 +493,6 @@ def nansum(values, axis=None, skipna=True, min_count=0, mask=None):
return _wrap_results(the_sum, dtype)


@disallow("M8", DatetimeTZDtype)
@bottleneck_switch()
def nanmean(values, axis=None, skipna=True, mask=None):
"""
Expand Down Expand Up @@ -552,7 +550,6 @@ def nanmean(values, axis=None, skipna=True, mask=None):
return _wrap_results(the_mean, dtype)


@disallow("M8")
@bottleneck_switch()
def nanmedian(values, axis=None, skipna=True, mask=None):
"""
Expand Down Expand Up @@ -585,8 +582,12 @@ def get_median(x):
return np.nanmedian(x[mask])

values, mask, dtype, dtype_max, _ = _get_values(values, skipna, mask=mask)
if not is_float_dtype(values):
values = values.astype("f8")
if not is_float_dtype(values.dtype):
try:
values = values.astype("f8")
except ValueError:
# e.g. "could not convert string to float: 'a'"
raise TypeError
if mask is not None:
values[mask] = np.nan

Expand Down
18 changes: 13 additions & 5 deletions pandas/tests/frame/test_analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,15 @@ def assert_stat_op_calc(
f = getattr(frame, opname)

if check_dates:
expected_warning = FutureWarning if opname in ["mean", "median"] else None
df = DataFrame({"b": date_range("1/1/2001", periods=2)})
result = getattr(df, opname)()
with tm.assert_produces_warning(expected_warning):
result = getattr(df, opname)()
assert isinstance(result, Series)

df["a"] = range(len(df))
result = getattr(df, opname)()
with tm.assert_produces_warning(expected_warning):
result = getattr(df, opname)()
assert isinstance(result, Series)
assert len(result)

Expand Down Expand Up @@ -1062,7 +1065,8 @@ def test_nunique(self):
def test_mean_mixed_datetime_numeric(self, tz):
# https://github.com/pandas-dev/pandas/issues/24752
df = pd.DataFrame({"A": [1, 1], "B": [pd.Timestamp("2000", tz=tz)] * 2})
result = df.mean()
with tm.assert_produces_warning(FutureWarning):
result = df.mean()
expected = pd.Series([1.0], index=["A"])
tm.assert_series_equal(result, expected)

Expand All @@ -1072,7 +1076,9 @@ def test_mean_excludes_datetimes(self, tz):
# Our long-term desired behavior is unclear, but the behavior in
# 0.24.0rc1 was buggy.
df = pd.DataFrame({"A": [pd.Timestamp("2000", tz=tz)] * 2})
result = df.mean()
with tm.assert_produces_warning(FutureWarning):
result = df.mean()

expected = pd.Series(dtype=np.float64)
tm.assert_series_equal(result, expected)

Expand Down Expand Up @@ -1458,7 +1464,9 @@ def test_mean_datetimelike(self):
expected = pd.Series({"A": 1.0})
tm.assert_series_equal(result, expected)

result = df.mean()
with tm.assert_produces_warning(FutureWarning):
# in the future datetime columns will be included
result = df.mean()
expected = pd.Series({"A": 1.0, "C": df.loc[1, "C"]})
tm.assert_series_equal(result, expected)

Expand Down
1 change: 0 additions & 1 deletion pandas/tests/test_nanops.py
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,6 @@ def prng(self):

class TestDatetime64NaNOps:
@pytest.mark.parametrize("tz", [None, "UTC"])
@pytest.mark.xfail(reason="disabled")
# Enabling mean changes the behavior of DataFrame.mean
# See https://github.com/pandas-dev/pandas/issues/24752
def test_nanmean(self, tz):
Expand Down