CLN: _wrap_applied_output

[rhshadrach]

The majority of this change is the result of three operations:

1. Move code that returns up top, to avoid the heavily nested structure.
2. Move variable creation as close as possible to where they are used. Previously, in certain cases computations were being done and then going unused.
3. Combine duplicated code.

There are two sections that I was able to entirely remove:

ping = self.grouper.groupings[0]
if len(keys) == ping.ngroups:
    key_index = ping.group_index
    key_index.name = key_names[0]

    key_lookup = Index(keys)
    indexer = key_lookup.get_indexer(key_index)

    # reorder the values
    values = [values[i] for i in indexer]

    # update due to the potential reorder
    first_not_none = next(com.not_none(*values), None)

and

# GH5788 instead of stacking; concat gets the
# dtypes correct
from pandas.core.reshape.concat import concat

result = concat(
    values,
    keys=key_index,
    names=key_index.names,
    axis=self.axis,
).unstack()
result.columns = index

For the single test that I touched, it was checking that the categorical dtype of an index was being dropped after a groupby. I don't believe that is the correct behavior - that the categorical dtype should remain. I checked groupby/categorical bugs and didn't find any issues this closes.

[WillAyd]

Can this just be an else statement? Or are there more types we handle than these + NDFrame?

[rhshadrach]

I think you're suggesting something like:

if isinstance(v, (np.ndarray, Index, Series)):
    ...
else:
    ...

The reason I have opted not to do this is that the if-block is exceedingly long, whereas the else-block is quite short. Doing it this way would result in a more nested rather than flat structure.

[rhshadrach]

Ack - sorry, I see what you're saying now. Ignore my previous response, will investigate.

[WillAyd]

Thanks for refactoring this - these code paths as I'm sure you've noticed are thorny, so cleanups here are much appreciated

[WillAyd]

Is there an open issue for this?

[rhshadrach]

I don't believe so. There is only one issue tagged with categorical, groupby, and apply which is not relevant. I also took a look through those tagged as categorical and groupby and didn't see anything either.

[WillAyd]

Can you add a whatsnew for this? Something along the lines of groupby apply will now maintain a CategoricalIndex (assuming that is now the case)

[rhshadrach]

Codecov is finding a lot in this method that isn't hit. I'm going to see what tests can be added or branches that can be removed.

[WillAyd]

Cool I think this is a nice refactor @jreback

[WillAyd]

Can you add a whatsnew for this? Something along the lines of groupby apply will now maintain a CategoricalIndex (assuming that is now the case)

[jbrockmendel]

The majority of this change is the result of three operations:

To the extent that you can split this into independent pieces, it will be easier to review.