Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v3.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,7 @@ Groupby/resample/rolling
- Bug in :meth:`.DataFrameGroupBy.groups` and :meth:`.SeriesGroupBy.groups` would fail when the groups were :class:`Categorical` with an NA value (:issue:`61356`)
- Bug in :meth:`.DataFrameGroupBy.groups` and :meth:`.SeriesGroupby.groups` that would not respect groupby argument ``dropna`` (:issue:`55919`)
- Bug in :meth:`.DataFrameGroupBy.median` where nat values gave an incorrect result. (:issue:`57926`)
- Bug in :meth:`.DataFrameGroupBy` reductions where boolean-valued inputs were mishandled in the Cython aggregation path (``_cython_agg_general``); adding an ``is_bool`` check fixes incorrect results for some bool inputs. (:issue:`62778`)
Copy link
Member

Choose a reason for hiding this comment

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

Only mention public things in the whatsnew. Also "Boolean-valued inputs" makes it sounds like there was an issue with True and False.

Suggested change
- Bug in :meth:`.DataFrameGroupBy` reductions where boolean-valued inputs were mishandled in the Cython aggregation path (``_cython_agg_general``); adding an ``is_bool`` check fixes incorrect results for some bool inputs. (:issue:`62778`)
- Bug in :meth:`.DataFrameGroupBy` reductions where non-Boolean values were allowed for the ``numeric_only`` argument; passing a non-Boolean value will now raise (:issue:`62778`)

Copy link
Author

Choose a reason for hiding this comment

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

Sure, let me correct that.

Copy link
Author

Choose a reason for hiding this comment

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

@rhshadrach Made the required changes!

- Bug in :meth:`.DataFrameGroupBy.quantile` when ``interpolation="nearest"`` is inconsistent with :meth:`DataFrame.quantile` (:issue:`47942`)
- Bug in :meth:`.Resampler.interpolate` on a :class:`DataFrame` with non-uniform sampling and/or indices not aligning with the resulting resampled index would result in wrong interpolation (:issue:`21351`)
- Bug in :meth:`.Series.rolling` when used with a :class:`.BaseIndexer` subclass and computing min/max (:issue:`46726`)
Expand Down
6 changes: 5 additions & 1 deletion pandas/core/groupby/groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class providing the base-class of operations.
is_object_dtype,
is_scalar,
is_string_dtype,
is_bool,
needs_i8_conversion,
pandas_dtype,
)
Expand Down Expand Up @@ -1754,7 +1755,10 @@ def _cython_agg_general(
**kwargs,
):
# Note: we never get here with how="ohlc" for DataFrameGroupBy;
# that goes through SeriesGroupBy
# that goes through SeriesGroupBy
Copy link
Member

Choose a reason for hiding this comment

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

Can you revert this change.

Copy link
Member

Choose a reason for hiding this comment

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

Still needs to be reverted in order to minimize the diff.


if not is_bool(numeric_only):
raise ValueError("numeric_only accepts only Boolean values")

data = self._get_data_to_aggregate(numeric_only=numeric_only, name=how)

Expand Down
24 changes: 24 additions & 0 deletions pandas/tests/groupby/test_reductions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1520,3 +1520,27 @@ def test_groupby_std_datetimelike():
exp_ser = Series([td1 * 2, td1, td1, td1, td4], index=np.arange(5))
expected = DataFrame({"A": exp_ser, "B": exp_ser, "C": exp_ser})
tm.assert_frame_equal(result, expected)

def test_mean_numeric_only_validates_bool():
"""
Test that numeric_only parameter only accepts boolean values.
See GH#62778
"""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""
Test that numeric_only parameter only accepts boolean values.
See GH#62778
"""
# GH#62778

df = pd.DataFrame({"A": range(5), "B": range(5)})

# These test cases should raise a ValueError
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this comment, it repeats the code.

msg = "numeric_only accepts only Boolean values"
with pytest.raises(ValueError, match=msg):
df.groupby(["A"]).mean(["B"])

with pytest.raises(ValueError, match=msg):
df.groupby(["A"]).mean(numeric_only="True")

with pytest.raises(ValueError, match=msg):
df.groupby(["A"]).mean(numeric_only=1)

# These test cases should work absolutely fine
df.groupby(["A"]).mean()
df.groupby(["A"]).mean(numeric_only=True)
df.groupby(["A"]).mean(numeric_only=False)
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove these; the test suite has many tests for numeric_only being specified or not specified, these are not increasing our test coverage.

Copy link
Author

Choose a reason for hiding this comment

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

So should I remove this whole function from the test file or just these 3 lines of code?

Copy link
Member

@rhshadrach rhshadrach Oct 26, 2025

Choose a reason for hiding this comment

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

Just these four lines.

Copy link
Author

Choose a reason for hiding this comment

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

@rhshadrach Made the suggested changes. I had made the changes in the comment inside groupby.py file but forgot to add it while doing git add, hence, it didn't show up in the changes here.

This version should be fine. Please let me know if any other changes are there.


Loading