ENH: pd.NamedAgg forwards *args and **kwargs to aggfunc #58852
Conversation
tomytp
changed the title
| @@ -108,7 +108,12 @@ | |||
| ScalarResult = TypeVar("ScalarResult") | |||
|
|
|||
|
|
|||
| class NamedAgg(NamedTuple): | |||
| class _BaseNamedAgg(NamedTuple): | |||
There was a problem hiding this comment.
What's the reason for introducing a new class here?
There was a problem hiding this comment.
That's because you cannot override new when inheriting from NamedTuple directly.
There was a problem hiding this comment.
So the most elegant way I found to override it while maintaining the NamedTuple properties was to Introduce a new class. Do you have a suggestion for another method to do it?
There was a problem hiding this comment.
the reference error for when I tried to inherit directly from NamedTuple is:
AttributeError: Cannot overwrite NamedTuple attribute __new__
There was a problem hiding this comment.
Ah that's unfortunate. This seems kind of hacky as a result though - maybe we can just drop the NamedTuple subclass? I think that was mostly developed that way as a convenience rather than an intentional design decision - I am not sure why NamedAgg would need to be "tuple-ish"
@rhshadrach might have more thoughts
There was a problem hiding this comment.
python/cpython#77258 has some relevant information, especially the linked discussion there to the issue in python/typing.
In any case, it looks to me like this implementation just ignores args / kwargs when the provided aggfunc is not a callable? If that's the case, then I'm -1 on this approach anyways.
There was a problem hiding this comment.
@rhshadrach I'll rethink the approach to include the function names passed as string.
I'm not sure I understand your thoughts on the necessity of keeping NamedArgs with tuple behavior. Could you clarify that?
TY
There was a problem hiding this comment.
I'm not sure I understand your thoughts on the necessity of keeping NamedArgs with tuple behavior. Could you clarify that?
I don't believe I shared any thoughts in this regard, just a link to some background information on why Python raises when you try to overwrite __new__.
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
Closing as stale. @tomytp - if you'd like to continue, comment here and we can reopen. |
doc/source/whatsnew/v3.0.0.rstfile if fixing a bug or adding a new feature.