From f7f0f65a3084b2109a2d53ca69dc0bbed3a5270c Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 3 Oct 2019 10:02:42 -0500 Subject: [PATCH 1/5] CLN: Specific exceptions instead of Exception x 2 --- pandas/core/groupby/groupby.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index e93ce3ce93164..eadfc357599db 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -11,6 +11,7 @@ class providing the base-class of operations. from contextlib import contextmanager import datetime from functools import partial, wraps +import re import types from typing import FrozenSet, List, Optional, Tuple, Type, Union @@ -638,10 +639,17 @@ def curried(x): try: return self.apply(curried_with_axis) - except Exception: - try: + except TypeError as err: + if "got an unexpected keyword argument 'axis'" in str(err): + # We need to use the `curried` instead of `curried_with_axis` + # Any exception other than needing to use `curried` + # rather than `curried_with_axis` gets re-raised. return self.apply(curried) - except Exception: + elif re.search( + "reduction operation '.*' not allowed for this dtype", str(err) + ): + # We don't have a cython implementation + # TODO: is the above comment accurate? # related to : GH3688 # try item-by-item @@ -653,7 +661,10 @@ def curried(x): return self._aggregate_item_by_item(name, *args, **kwargs) except AttributeError: # e.g. SparseArray has no flags attr + # FIXME: 'SeriesGroupBy' object has no attribute '_aggregate_item_by_item' + # occurs in idxmax() case in tests.groupby.test_function.test_non_cython_api raise ValueError + raise return wrapper From c07be053fc681ec9d617f99346e427a07a20a082 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 3 Oct 2019 11:39:43 -0500 Subject: [PATCH 2/5] Pre-empt error altogether --- pandas/core/groupby/groupby.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index eadfc357599db..4e9fdcba927a6 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -618,34 +618,30 @@ def _make_wrapper(self, name): def wrapper(*args, **kwargs): # a little trickery for aggregation functions that need an axis # argument - kwargs_with_axis = kwargs.copy() - if "axis" not in kwargs_with_axis or kwargs_with_axis["axis"] is None: - kwargs_with_axis["axis"] = self.axis - - def curried_with_axis(x): - return f(x, *args, **kwargs_with_axis) + kwargs2 = kwargs.copy() def curried(x): - return f(x, *args, **kwargs) + return f(x, *args, **kwargs2) # preserve the name so we can detect it when calling plot methods, # to avoid duplicates - curried.__name__ = curried_with_axis.__name__ = name + curried.__name__ = name # special case otherwise extra plots are created when catching the # exception below if name in base.plotting_methods: return self.apply(curried) + import inspect + sig = inspect.signature(f) + if "axis" in sig.parameters: + if kwargs.get("axis", None) is None: + kwargs2["axis"] = self.axis + try: - return self.apply(curried_with_axis) + return self.apply(curried) except TypeError as err: - if "got an unexpected keyword argument 'axis'" in str(err): - # We need to use the `curried` instead of `curried_with_axis` - # Any exception other than needing to use `curried` - # rather than `curried_with_axis` gets re-raised. - return self.apply(curried) - elif re.search( + if re.search( "reduction operation '.*' not allowed for this dtype", str(err) ): # We don't have a cython implementation @@ -661,11 +657,13 @@ def curried(x): return self._aggregate_item_by_item(name, *args, **kwargs) except AttributeError: # e.g. SparseArray has no flags attr - # FIXME: 'SeriesGroupBy' object has no attribute '_aggregate_item_by_item' - # occurs in idxmax() case in tests.groupby.test_function.test_non_cython_api + # FIXME: 'SeriesGroupBy' no attribute '_aggregate_item_by_item' + # occurs in idxmax() case + # in tests.groupby.test_function.test_non_cython_api raise ValueError raise + wrapper.__name__ = name return wrapper def get_group(self, name, obj=None): From 0dc950325a12f785e1c7c9a64d310aeb710c0b4b Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 3 Oct 2019 11:43:10 -0500 Subject: [PATCH 3/5] dedent --- pandas/core/groupby/groupby.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 4e9fdcba927a6..97a4252a91eb3 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -641,27 +641,27 @@ def curried(x): try: return self.apply(curried) except TypeError as err: - if re.search( + if not re.search( "reduction operation '.*' not allowed for this dtype", str(err) ): # We don't have a cython implementation # TODO: is the above comment accurate? - - # related to : GH3688 - # try item-by-item - # this can be called recursively, so need to raise - # ValueError - # if we don't have this method to indicated to aggregate to - # mark this column as an error - try: - return self._aggregate_item_by_item(name, *args, **kwargs) - except AttributeError: - # e.g. SparseArray has no flags attr - # FIXME: 'SeriesGroupBy' no attribute '_aggregate_item_by_item' - # occurs in idxmax() case - # in tests.groupby.test_function.test_non_cython_api - raise ValueError - raise + raise + + # related to : GH3688 + # try item-by-item + # this can be called recursively, so need to raise + # ValueError + # if we don't have this method to indicated to aggregate to + # mark this column as an error + try: + return self._aggregate_item_by_item(name, *args, **kwargs) + except AttributeError: + # e.g. SparseArray has no flags attr + # FIXME: 'SeriesGroupBy' has no attribute '_aggregate_item_by_item' + # occurs in idxmax() case + # in tests.groupby.test_function.test_non_cython_api + raise ValueError wrapper.__name__ = name return wrapper From 7c023791b96584a53f0d980c475b08362e867795 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 3 Oct 2019 12:35:26 -0500 Subject: [PATCH 4/5] blackify --- pandas/core/groupby/groupby.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 97a4252a91eb3..90a50fe2fe57b 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -633,6 +633,7 @@ def curried(x): return self.apply(curried) import inspect + sig = inspect.signature(f) if "axis" in sig.parameters: if kwargs.get("axis", None) is None: From 8d8fed9893dec54d7b82957f97187c2f8024a79d Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Mon, 7 Oct 2019 19:44:09 -0700 Subject: [PATCH 5/5] add axis earlier --- pandas/core/groupby/groupby.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 90a50fe2fe57b..98087fd29cc50 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -11,6 +11,7 @@ class providing the base-class of operations. from contextlib import contextmanager import datetime from functools import partial, wraps +import inspect import re import types from typing import FrozenSet, List, Optional, Tuple, Type, Union @@ -614,14 +615,17 @@ def _make_wrapper(self, name): return self.apply(lambda self: getattr(self, name)) f = getattr(type(self._selected_obj), name) + sig = inspect.signature(f) def wrapper(*args, **kwargs): # a little trickery for aggregation functions that need an axis # argument - kwargs2 = kwargs.copy() + if "axis" in sig.parameters: + if kwargs.get("axis", None) is None: + kwargs["axis"] = self.axis def curried(x): - return f(x, *args, **kwargs2) + return f(x, *args, **kwargs) # preserve the name so we can detect it when calling plot methods, # to avoid duplicates @@ -632,13 +636,6 @@ def curried(x): if name in base.plotting_methods: return self.apply(curried) - import inspect - - sig = inspect.signature(f) - if "axis" in sig.parameters: - if kwargs.get("axis", None) is None: - kwargs2["axis"] = self.axis - try: return self.apply(curried) except TypeError as err: