Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
BUG: passing str to GroupBy.apply #42021
Conversation
pandas/core/groupby/groupby.py
Outdated
| @@ -1245,6 +1245,16 @@ def f(g): | ||
| "func must be a callable if args or kwargs are supplied" | ||
| ) | ||
| else: | ||
| if isinstance(func, str): |
pandas/core/groupby/groupby.py
Outdated
| if isinstance(func, str): | ||
| if hasattr(self, func): | ||
| res = getattr(self, func) | ||
| if inspect.isfunction(res) or inspect.ismethod(res): |
pandas/core/groupby/groupby.py
Outdated
| res = getattr(self, func) | ||
| if inspect.isfunction(res) or inspect.ismethod(res): | ||
| return res() | ||
| return res |
There was a problem hiding this comment.
why are you returning? does any test actually hit this? should just fall thru no?
There was a problem hiding this comment.
yes, several of the test_empty_groupby tests get here
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
|
lgtm, can you merge master one more time. |
|
Thanks @jbrockmendel |
BUG: passing str to GroupBy.apply (pandas-dev#42021)
The text was updated successfully, but these errors were encountered: