BUG: passing str to GroupBy.apply

[jbrockmendel]

• closes #xxxx
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

[jreback]

can this be an elif

[jbrockmendel]

Updated +greenish

[jreback]

if callable(res) ?

[jreback]

why are you returning? does any test actually hit this? should just fall thru no?

[jbrockmendel]

yes, several of the test_empty_groupby tests get here

[rhshadrach]

If we only raise, is there any issue? I.e. to just do

elif isinstance(func, str) and not hasattr(self, func):
    raise TypeError(f"apply func should be callable, not '{func}'")

Or perhaps there is some benefit to adding the call to res() here?

[jbrockmendel]

you mean return res unconditionally instead of returning res() in some cases? that would end up returning obj.method_name instead of obj.method_name(), which seems like a pretty weird thing to return from an apply call

[rhshadrach]

Not quite - I put my suggestion (that doesn't work) below in case you're curious, but see now this PR isn't solely fixing the case of an invalid str argument, but also addressing the # FIXME: apply goes through different code path. This makes sense now, +1.

        if args or kwargs:
            if callable(func):

                @wraps(func)
                def f(g):
                    with np.errstate(all="ignore"):
                        return func(g, *args, **kwargs)

            elif hasattr(nanops, "nan" + func):
                # TODO: should we wrap this in to e.g. _is_builtin_func?
                f = getattr(nanops, "nan" + func)

            else:
                raise ValueError(
                    "func must be a callable if args or kwargs are supplied"
                )
        elif isinstance(func, str) and not hasattr(self, func):
            raise TypeError(f"apply func should be callable, not '{func}'")
        else:
            f = func

[rhshadrach]

lgtm, can you merge master one more time.

[rhshadrach]

Thanks @jbrockmendel