Skip to content

Update MultiIndex checks #29494

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 2 commits into from
Nov 13, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pandas/_libs/reduction.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -512,8 +512,8 @@ def apply_frame_axis0(object frame, object f, object names,
object piece
dict item_cache

if frame.index._has_complex_internals:
raise InvalidApply('Cannot modify frame index internals')
# We have already checked that we don't have a MultiIndex before calling
assert frame.index.nlevels == 1

results = []

Expand Down Expand Up @@ -644,7 +644,7 @@ def compute_reduction(arr, f, axis=0, dummy=None, labels=None):

if labels is not None:
# Caller is responsible for ensuring we don't have MultiIndex
assert not labels._has_complex_internals
assert labels.nlevels == 1
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to do an isinstance check here? Not a strong blocker usage is mixed; might be nice to settle on one way of doing this

Copy link
Member Author

Choose a reason for hiding this comment

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

because we're in cython and dont want to import non-cython


# pass as an ndarray/ExtensionArray
labels = labels._values
Expand Down
21 changes: 10 additions & 11 deletions pandas/core/groupby/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,27 +159,26 @@ def apply(self, f, data, axis: int = 0):
com.get_callable_name(f) not in base.plotting_methods
and hasattr(splitter, "fast_apply")
and axis == 0
# with MultiIndex, apply_frame_axis0 would raise InvalidApply
# TODO: can we make this check prettier?
and not sdata.index._has_complex_internals
# apply_frame_axis0 doesn't allow MultiIndex
and not isinstance(sdata.index, MultiIndex)
):
try:
result_values, mutated = splitter.fast_apply(f, group_keys)

# If the fast apply path could be used we can return here.
# Otherwise we need to fall back to the slow implementation.
if len(result_values) == len(group_keys):
return group_keys, result_values, mutated

except libreduction.InvalidApply as err:
# Cannot fast apply on MultiIndex (_has_complex_internals).
# This Exception is also raised if `f` triggers an exception
# This Exception is raised if `f` triggers an exception
# but it is preferable to raise the exception in Python.
if "Let this error raise above us" not in str(err):
# TODO: can we infer anything about whether this is
# worth-retrying in pure-python?
raise

else:
# If the fast apply path could be used we can return here.
# Otherwise we need to fall back to the slow implementation.
if len(result_values) == len(group_keys):
return group_keys, result_values, mutated

for key, (i, group) in zip(group_keys, splitter):
object.__setattr__(group, "name", key)

Expand Down Expand Up @@ -590,7 +589,7 @@ def agg_series(self, obj: Series, func):
# TODO: is the datetime64tz case supposed to go through here?
return self._aggregate_series_pure_python(obj, func)

elif obj.index._has_complex_internals:
elif isinstance(obj.index, MultiIndex):
# MultiIndex; Pre-empt TypeError in _aggregate_series_fast
return self._aggregate_series_pure_python(obj, func)

Expand Down