Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ENH: Support nested renaming / selection #26399
ENH: Support nested renaming / selection #26399
Changes from 5 commits
aa43cf6
8bd8e31
10c8f40
2e52653
06a86ec
9e636c1
14f66e6
2c3d11a
cdf9373
2c544f0
c0cd575
386cca1
2f6e1dc
6d8a18a
6c1f567
bcc63f5
769a909
1da90d4
a028f48
0ddd51f
42e69a1
769d7d3
1cee0e2
6369eb1
02d7169
eb9ba8f
7df14d7
cf8db51
9501e82
d65afe4
25dca1a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not show the example in a mixed form (as this is something we really don't want to recommend I think?). I would maybe just show it twice, eg first with tuples and then with comment
# using more explicit syntax
with namedtuple. Then it also shows that both result in the same.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these are technically identifiers instead of keywords
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated to this PR, but there is actually a note a few lines below about "the ordering of the output columns is non-deterministic" that can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm couldn't we just do this now since
order
keeps track of that for us?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recall if I checked, but I thought we needed this to ensure that the order of the arguments is respected in
_python_agg_general
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why doesn't this work? These are not lambda functions. Do you mean that the ("selected colname", "aggfunc
__name__
") pairs must be all unique?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to add, that ideally:
__name__
") pairs should not have to be all unique__name__
") pairs must be all uniqueIf the above holds, then using multiple lambdas and/or multiple partials based on the same base function would work and this would solve the issues mentioned in the initial post of #18366.
I suppose that this implementation-related limitation is the one you were referring to at the end of your comment in #18366 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This could perhaps be relaxed in the future.