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

REGR: Groupby methods not supporting numba raising TypeError when the… #55586

Merged
merged 3 commits into from Oct 25, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.1.2.rst
Expand Up @@ -17,6 +17,7 @@ Fixed regressions
- Fixed regression in :meth:`DataFrame.join` where result has missing values and dtype is arrow backed string (:issue:`55348`)
- Fixed regression in :meth:`DataFrame.resample` which was extrapolating back to ``origin`` when ``origin`` was outside its bounds (:issue:`55064`)
- Fixed regression in :meth:`DataFrame.sort_index` which was not sorting correctly when the index was a sliced :class:`MultiIndex` (:issue:`55379`)
- Fixed regression in :meth:`DataFrameGroupBy.agg` and :meth:`SeriesGroupBy.agg` where if the option ``compute.use_numba`` was set to True, groupby methods not supported by the numba engine would raise a ``TypeError`` (:issue:`55520`)

.. ---------------------------------------------------------------------------
.. _whatsnew_212.bug_fixes:
Expand Down
6 changes: 4 additions & 2 deletions pandas/core/groupby/generic.py
Expand Up @@ -236,9 +236,11 @@ def aggregate(self, func=None, *args, engine=None, engine_kwargs=None, **kwargs)
kwargs = {}

if isinstance(func, str):
if maybe_use_numba(engine):
if maybe_use_numba(engine) and engine is not None:
# Not all agg functions support numba, only propagate numba kwargs
# if user asks for numba
# if user asks for numba, and engine is not None
# (if engine is None, the called function will handle the case where
# numba is requested via the global option)
kwargs["engine"] = engine
kwargs["engine_kwargs"] = engine_kwargs
Copy link
Member

Choose a reason for hiding this comment

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

If engine is None and engine_kwargs is not None, don't the engine_kwargs end up getting ignored here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I see.

The solution would be to always pass engine_kwargs then?
Since the user has to explicitly do this, then it would be their fault then if they pass in engine_kwargs to a function that doesn't support it.

Copy link
Member

Choose a reason for hiding this comment

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

As long as engine_kwargs is not None, yes, that sounds right to me.

return getattr(self, func)(*args, **kwargs)
Expand Down
12 changes: 12 additions & 0 deletions pandas/tests/groupby/test_numba.py
Expand Up @@ -3,6 +3,7 @@
from pandas import (
DataFrame,
Series,
option_context,
)
import pandas._testing as tm

Expand Down Expand Up @@ -66,3 +67,14 @@ def test_axis_1_unsupported(self, numba_supported_reductions):
gb = df.groupby("a", axis=1)
with pytest.raises(NotImplementedError, match="axis=1"):
getattr(gb, func)(engine="numba", **kwargs)

def test_no_engine_doesnt_raise(self):
# GH55520
df = DataFrame({"a": [3, 2, 3, 2], "b": range(4), "c": range(1, 5)})
gb = df.groupby("a")
# Make sure behavior of functions w/out engine argument don't raise
# when the global use_numba option is set
with option_context("compute.use_numba", True):
res = gb.agg({"b": "first"})
expected = gb.agg({"b": "first"})
tm.assert_frame_equal(res, expected)