From 791954c3e6d4f93732221fa96ecac5ce76d5ee51 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 14 Nov 2019 19:39:38 -0800 Subject: [PATCH 1/3] REF: no need to catch Exception in choose_path --- pandas/core/groupby/generic.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 002d8640f109d..0915dd1372dc9 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1412,19 +1412,12 @@ def _define_paths(self, func, *args, **kwargs): ) return fast_path, slow_path - def _choose_path(self, fast_path, slow_path, group): + def _choose_path(self, fast_path: Callable, slow_path: Callable, group: DataFrame): path = slow_path res = slow_path(group) # if we make it here, test if we can use the fast path - try: - res_fast = fast_path(group) - except AssertionError: - raise - except Exception: - # Hard to know ex-ante what exceptions `fast_path` might raise - # TODO: no test cases get here - return path, res + res_fast = fast_path(group) # verify fast path does not change columns (and names), otherwise # its results cannot be joined with those of the slow path From 2b41e05e1e2d83eff412b3e75333e411d43cb5c1 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Tue, 19 Nov 2019 09:19:23 -0800 Subject: [PATCH 2/3] restore catch, add test --- pandas/core/groupby/generic.py | 9 ++++++++- pandas/tests/groupby/test_transform.py | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 8ef79f1c7985e..e4a6066857abd 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1401,7 +1401,14 @@ def _choose_path(self, fast_path: Callable, slow_path: Callable, group: DataFram res = slow_path(group) # if we make it here, test if we can use the fast path - res_fast = fast_path(group) + try: + res_fast = fast_path(group) + except AssertionError: + raise + except Exception: + # GH#29631 For user-defined function, we cant predict what may be + # raised; see test_transform.test_transform_fastpath_raises + return path, res # verify fast path does not change columns (and names), otherwise # its results cannot be joined with those of the slow path diff --git a/pandas/tests/groupby/test_transform.py b/pandas/tests/groupby/test_transform.py index db44a4a57230c..ca2b841869919 100644 --- a/pandas/tests/groupby/test_transform.py +++ b/pandas/tests/groupby/test_transform.py @@ -1073,3 +1073,29 @@ def test_transform_lambda_with_datetimetz(): name="time", ) tm.assert_series_equal(result, expected) + + +def test_transform_fastpath_raises(): + # GH#29631 case where fastpath defined in groupby.generic _choose_path + # raises, but slow_path does not + + df = pd.DataFrame({"A": [1, 1, 2, 2], "B": [1, -1, 1, 2]}) + gb = df.groupby("A") + + def replace(g): + mask = g < 0 + return g.where(mask, g[~mask].mean()) + + # Check that the fastpath raises, see _transform_general + obj = gb._obj_with_exclusions + gen = gb.grouper.get_iterator(obj, axis=gb.axis) + fast_path, slow_path = gb._define_paths(replace) + _, group = next(gen) + + with pytest.raises(ValueError, match="Must specify axis"): + fast_path(group) + + result = gb.transform(replace) + + expected = pd.DataFrame([1, -1, 1.5, 1.5], columns=["B"]) + tm.assert_frame_equal(result, expected) From b307531f557efad14539acdd0c02f9a0b66a524f Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Tue, 19 Nov 2019 14:19:35 -0800 Subject: [PATCH 3/3] simpler function --- pandas/tests/groupby/test_transform.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/pandas/tests/groupby/test_transform.py b/pandas/tests/groupby/test_transform.py index ca2b841869919..3d9a349d94e10 100644 --- a/pandas/tests/groupby/test_transform.py +++ b/pandas/tests/groupby/test_transform.py @@ -1082,20 +1082,24 @@ def test_transform_fastpath_raises(): df = pd.DataFrame({"A": [1, 1, 2, 2], "B": [1, -1, 1, 2]}) gb = df.groupby("A") - def replace(g): - mask = g < 0 - return g.where(mask, g[~mask].mean()) + def func(grp): + # we want a function such that func(frame) fails but func.apply(frame) + # works + if grp.ndim == 2: + # Ensure that fast_path fails + raise NotImplementedError("Don't cross the streams") + return grp * 2 # Check that the fastpath raises, see _transform_general obj = gb._obj_with_exclusions gen = gb.grouper.get_iterator(obj, axis=gb.axis) - fast_path, slow_path = gb._define_paths(replace) + fast_path, slow_path = gb._define_paths(func) _, group = next(gen) - with pytest.raises(ValueError, match="Must specify axis"): + with pytest.raises(NotImplementedError, match="Don't cross the streams"): fast_path(group) - result = gb.transform(replace) + result = gb.transform(func) - expected = pd.DataFrame([1, -1, 1.5, 1.5], columns=["B"]) + expected = pd.DataFrame([2, -2, 2, 4], columns=["B"]) tm.assert_frame_equal(result, expected)