Skip to content
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.1.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ Groupby/resample/rolling
grouped :class:`Series` or :class:`DataFrame` was a :class:`DatetimeIndex`, :class:`TimedeltaIndex`
or :class:`PeriodIndex`, and the ``groupby`` method was given a function as its first argument,
the function operated on the whole index rather than each element of the index. (:issue:`51979`)
- Bug in :meth:`GroupBy.groups` with a datetime key in conjunction with another key produced incorrect number of group keys (:issue:`51158`)
- Bug in :meth:`GroupBy.var` failing to raise ``TypeError`` when called with datetime64 or :class:`PeriodDtype` values (:issue:`52128`)
-

Expand Down
3 changes: 3 additions & 0 deletions pandas/core/groupby/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -1186,6 +1186,9 @@ def groups(self):
}
return result

def __iter__(self) -> Iterator[Hashable]:
return iter(self.groupings[0].grouping_vector)
Copy link
Member

Choose a reason for hiding this comment

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

why is the parent class method wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I forgot to add description. Please, see the description of the PR.

Copy link
Member

@rhshadrach rhshadrach Mar 18, 2023

Choose a reason for hiding this comment

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

I wonder if it would be better to refactor the indices here to align better with the base class rather than override iter. But I'm not certain if that's a good idea and I'm okay with this fix. cc @jbrockmendel

Copy link
Member

Choose a reason for hiding this comment

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

@jbrockmendel - friendly ping.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the delay. i had in mind something more like @rhshadrach's suggestion, that needing this here might indicate that the self.indices (which the parent class method iterates over) might be wrong


@property
def nkeys(self) -> int:
# still matches len(self.groupings), but we can hard-code
Expand Down
23 changes: 23 additions & 0 deletions pandas/tests/groupby/test_grouping.py
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,29 @@ def test_groupby_grouper_f_sanity_checked(self):
expected.index.freq = None
tm.assert_series_equal(result, expected)

def test_groupby_with_datetime_key(self):
# GH 51158
df = DataFrame(
{
"id": ["a", "b"] * 3,
"b": date_range("2000-01-01", "2000-01-03", freq="9H"),
}
)
grouper = Grouper(key="b", freq="D")
gb = df.groupby([grouper, "id"])

# test number of groups
expected = {
(Timestamp("2000-01-01"), "a"): [0, 2],
(Timestamp("2000-01-01"), "b"): [1],
(Timestamp("2000-01-02"), "a"): [4],
(Timestamp("2000-01-02"), "b"): [3, 5],
}
tm.assert_dict_equal(gb.groups, expected)

# test number of group keys
assert len(gb.groups.keys()) == 4

def test_grouping_error_on_multidim_input(self, df):
msg = "Grouper for '<class 'pandas.core.frame.DataFrame'>' not 1-dimensional"
with pytest.raises(ValueError, match=msg):
Expand Down