Skip to content

CLN: move small bits outside of try/excepts #28962

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

Merged
merged 5 commits into from
Oct 16, 2019
Merged
Show file tree
Hide file tree
Changes from all 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: 0 additions & 1 deletion pandas/_libs/algos_take_helper.pxi.in
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ cdef _take_2d(ndarray[take_t, ndim=2] values, object idx):
Py_ssize_t i, j, N, K
ndarray[Py_ssize_t, ndim=2, cast=True] indexer = idx
ndarray[take_t, ndim=2] result
object val

N, K = (<object>values).shape

Expand Down
8 changes: 3 additions & 5 deletions pandas/core/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ def aggregate(self, func, *args, **kwargs):

agg = aggregate

def _try_aggregate_string_function(self, arg, *args, **kwargs):
def _try_aggregate_string_function(self, arg: str, *args, **kwargs):
"""
if arg is a string, then try to operate on it:
- try to find a function (or attribute) on ourselves
Expand All @@ -292,12 +292,10 @@ def _try_aggregate_string_function(self, arg, *args, **kwargs):

f = getattr(np, arg, None)
if f is not None:
try:
if hasattr(self, "__array__"):
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to account for __array_ufunc__ or __array_function__ here as well? @TomAugspurger might want to take a look

Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, this change is OK since we're checking if self has __array__, which our Index / Series objects have (in addition to array_ufunc)

# in particular exclude Window
return f(self, *args, **kwargs)

except (AttributeError, TypeError):
pass

raise AttributeError(
"'{arg}' is not a valid function for "
"'{cls}' object".format(arg=arg, cls=type(self).__name__)
Expand Down
13 changes: 9 additions & 4 deletions pandas/core/groupby/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,7 @@ def _cython_agg_blocks(self, how, alt=None, numeric_only=True, min_count=-1):
if alt is None:
# we cannot perform the operation
# in an alternate way, exclude the block
assert how == "ohlc"
deleted_items.append(locs)
continue

Expand Down Expand Up @@ -1025,17 +1026,20 @@ def _aggregate_frame(self, func, *args, **kwargs):
if axis != obj._info_axis_number:
try:
for name, data in self:
result[name] = self._try_cast(func(data, *args, **kwargs), data)
fres = func(data, *args, **kwargs)
result[name] = self._try_cast(fres, data)
except Exception:
return self._aggregate_item_by_item(func, *args, **kwargs)
else:
for name in self.indices:
data = self.get_group(name, obj=obj)
try:
data = self.get_group(name, obj=obj)
result[name] = self._try_cast(func(data, *args, **kwargs), data)
fres = func(data, *args, **kwargs)
except Exception:
wrapper = lambda x: func(x, *args, **kwargs)
result[name] = data.apply(wrapper, axis=axis)
else:
result[name] = self._try_cast(fres, data)

return self._wrap_frame_output(result, obj)

Expand Down Expand Up @@ -1410,9 +1414,10 @@ def _transform_item_by_item(self, obj, wrapper):
for i, col in enumerate(obj):
try:
output[col] = self[col].transform(wrapper)
inds.append(i)
except Exception:
pass
else:
inds.append(i)

if len(output) == 0:
raise TypeError("Transform function invalid for data types")
Expand Down
12 changes: 3 additions & 9 deletions pandas/core/groupby/groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -598,14 +598,7 @@ def pipe(self, func, *args, **kwargs):
plot = property(GroupByPlot)

def _make_wrapper(self, name):
if name not in self._apply_whitelist:
Copy link
Member

Choose a reason for hiding this comment

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

Nice cleanup. At a glance this looks like a pretty interesting factory...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yah, this used to be called from within __gettattr__ (and exec was involved) Recently changed to be called from properties generated directly from apply_whitelist, so this check is no longer necessary

is_callable = callable(getattr(self._selected_obj, name, None))
kind = " callable " if is_callable else " "
msg = (
"Cannot access{0}attribute {1!r} of {2!r} objects, try "
"using the 'apply' method".format(kind, name, type(self).__name__)
)
raise AttributeError(msg)
assert name in self._apply_whitelist

self._set_group_selection()

Expand Down Expand Up @@ -919,9 +912,10 @@ def _python_agg_general(self, func, *args, **kwargs):
for name, obj in self._iterate_slices():
try:
result, counts = self.grouper.agg_series(obj, f)
output[name] = self._try_cast(result, obj, numeric_only=True)
except TypeError:
continue
else:
output[name] = self._try_cast(result, obj, numeric_only=True)

if len(output) == 0:
return self._python_apply_general(f)
Expand Down