Skip to content

CLN: no longer need to catch AttributeError, IndexError #29591

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 1 commit into from
Nov 14, 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: 1 addition & 0 deletions pandas/_libs/reduction.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ cdef class SeriesBinGrouper(_BaseGrouper):
def __init__(self, object series, object f, object bins, object dummy):

assert dummy is not None # always obj[:0]
assert len(bins) > 0 # otherwise we get IndexError in get_result

self.bins = bins
self.f = f
Expand Down
5 changes: 1 addition & 4 deletions pandas/core/groupby/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,7 @@ def aggregate(self, func=None, *args, **kwargs):

try:
return self._python_agg_general(func, *args, **kwargs)
except (ValueError, KeyError, AttributeError, IndexError):
# TODO: IndexError can be removed here following GH#29106
# TODO: AttributeError is caused by _index_data hijinx in
# libreduction, can be removed after GH#29160
Copy link
Member

Choose a reason for hiding this comment

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

Why can this AttributeError now be removed? (the mentioned PR wasn't merged)
It's solved in a different way?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was fixed as a side-effect in #29548. It turned out SeriesGrouper was incorrectly failing to update _index_data, only SeriesBinGrouper was doing it correctly.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, so it was the _data -> _index_data change there. I see!

except (ValueError, KeyError):
# TODO: KeyError is raised in _python_agg_general,
# see see test_groupby.test_basic
result = self._aggregate_named(func, *args, **kwargs)
Expand Down
8 changes: 6 additions & 2 deletions pandas/core/groupby/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,10 @@ def __init__(
self.mutated = mutated
self.indexer = indexer

# These lengths must match, otherwise we could call agg_series
# with empty self.bins, which would raise in libreduction.
assert len(self.binlabels) == len(self.bins)

@cache_readonly
def groups(self):
""" dict {group name -> group labels} """
Expand Down Expand Up @@ -828,10 +832,10 @@ def groupings(self) -> "List[grouper.Grouping]":
def agg_series(self, obj: Series, func):
# Caller is responsible for checking ngroups != 0
assert self.ngroups != 0
assert len(self.bins) > 0 # otherwise we'd get IndexError in get_result

if is_extension_array_dtype(obj.dtype):
# pre-empty SeriesBinGrouper from raising TypeError
# TODO: watch out, this can return None
# pre-empt SeriesBinGrouper from raising TypeError
return self._aggregate_series_pure_python(obj, func)

dummy = obj[:0]
Expand Down
1 change: 1 addition & 0 deletions pandas/core/resample.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ def _get_binner(self):
"""

binner, bins, binlabels = self._get_binner_for_time()
assert len(bins) == len(binlabels)
bin_grouper = BinGrouper(bins, binlabels, indexer=self.groupby.indexer)
return binner, bin_grouper

Expand Down