Skip to content

REGR: Fix TypeError in groupby min / max of period column #31477

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

Merged
merged 13 commits into from
Feb 4, 2020
Merged

REGR: Fix TypeError in groupby min / max of period column #31477

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
18508e5
Add test
Jan 31, 2020
a8bc3a8
Check for period dtype
Jan 31, 2020
23b7ba6
Release note
Jan 31, 2020
46bb8dc
Blacken
Jan 31, 2020
fd0e79b
Get rid of comma
Jan 31, 2020
9f063b3
Move release note
Jan 31, 2020
f973037
Move test
Jan 31, 2020
fc301d6
Add frame test
Jan 31, 2020
2b26a5c
Merge branch 'master' into period-bug
Jan 31, 2020
a7b60ef
Add comment
Feb 1, 2020
6bba2f5
Merge branch 'master' into period-bug
Feb 2, 2020
e28bb0b
Merge branch 'master' into period-bug
Feb 4, 2020
61b1016
whatsnew
TomAugspurger Feb 4, 2020
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.0.1.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ Plotting
Groupby/resample/rolling
^^^^^^^^^^^^^^^^^^^^^^^^

- Bug in :meth:`DataFrame.groupby` whereby taking the minimum or maximum of a column with period dtype would raise a ``TypeError``. (:issue:`31471`)
-
-

Expand Down
8 changes: 7 additions & 1 deletion pandas/core/groupby/ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
is_extension_array_dtype,
is_integer_dtype,
is_numeric_dtype,
is_period_dtype,
is_sparse,
is_timedelta64_dtype,
needs_i8_conversion,
Expand Down Expand Up @@ -567,7 +568,12 @@ def _cython_operation(
if swapped:
result = result.swapaxes(0, axis)

if is_datetime64tz_dtype(orig_values.dtype):
if is_datetime64tz_dtype(orig_values.dtype) or is_period_dtype(
orig_values.dtype
):
# We need to use the constructors directly for these dtypes
# since numpy won't recognize them
# https://github.com/pandas-dev/pandas/issues/31471
result = type(orig_values)(result.astype(np.int64), dtype=orig_values.dtype)
elif is_datetimelike and kind == "aggregate":
result = result.astype(orig_values.dtype)
Expand Down
28 changes: 28 additions & 0 deletions pandas/tests/groupby/aggregate/test_aggregate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,34 @@ def aggfunc(x):
tm.assert_frame_equal(result, expected)


@pytest.mark.parametrize("func", ["min", "max"])
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move these to test_timegrouper.py (same dir)

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we're grouping by an int column and only aggregating the period column (not using a time grouper), does the current location make more sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

This location seems OK since we're grouping by integers, not time.

def test_groupby_aggregate_period_column(func):
# GH 31471
groups = [1, 2]
periods = pd.period_range("2020", periods=2, freq="Y")
df = pd.DataFrame({"a": groups, "b": periods})

result = getattr(df.groupby("a")["b"], func)()
idx = pd.Int64Index([1, 2], name="a")
expected = pd.Series(periods, index=idx, name="b")

tm.assert_series_equal(result, expected)


@pytest.mark.parametrize("func", ["min", "max"])
def test_groupby_aggregate_period_frame(func):
# GH 31471
groups = [1, 2]
periods = pd.period_range("2020", periods=2, freq="Y")
df = pd.DataFrame({"a": groups, "b": periods})

result = getattr(df.groupby("a"), func)()
idx = pd.Int64Index([1, 2], name="a")
expected = pd.DataFrame({"b": periods}, index=idx)

tm.assert_frame_equal(result, expected)


class TestLambdaMangling:
def test_basic(self):
df = pd.DataFrame({"A": [0, 0, 1, 1], "B": [1, 2, 3, 4]})
Expand Down