-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-14608: [Python] Provide access to hash_aggregate functions through a Table.group_by method #11624
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
ARROW-14608: [Python] Provide access to hash_aggregate functions through a Table.group_by method #11624
Changes from all commits
67f287c
039b8de
2cfc926
9329bcf
d04716e
c70cbc6
a32a314
d19f61d
9e2c16f
05aaafb
30b64fe
8ec3153
25e6276
94c64e4
76fff1b
dfecba1
111f258
1bc4fd4
712dc94
c9bd87d
73a60d4
0126d5c
990e674
3e06ddf
45af478
be82014
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ Classes | |
| ChunkedArray | ||
| RecordBatch | ||
| Table | ||
| TableGroupBy | ||
|
|
||
| .. _api.tensor: | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,7 @@ | |
| function_registry, | ||
| get_function, | ||
| list_functions, | ||
| _group_by | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reaons for exposing this publicly? Is this just a leftover from previous attempts?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's to make it available when Given that there are many more internal functions in the |
||
| ) | ||
|
|
||
| import inspect | ||
|
|
||
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.
We can also make this a public function in the compute module?
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.
Not sure, should we? I made it internal because we plan to replace this with the exec engine on long term, so I guess that the
Table.group_byimplementation will switch to use something different in the future.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.
The same could be done for a
pyarrow.computefunction? (it doesn't map 1:1 to a C++ kernel anyway)For me one reason to put it in the compute functions as a
pc.group_by(table, keys, ...)is to sidestep the 1-step vs 2-step API discussion for the method a bit. For a function in compute, I think it's totally fine to be a one step function