-
Notifications
You must be signed in to change notification settings - Fork 48
feat: bigframes.bigquery.array_agg(SeriesGroupBy|DataFrameGroupby) #663
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
Conversation
6e92dd6
to
c30464f
Compare
f703d45
to
333d0ac
Compare
1c44cef
to
e2b0854
Compare
824c27b
to
c5cc12e
Compare
c5cc12e
to
3e071ad
Compare
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.
A nit regarding the helper function naming, but otherwise looks good!
bigframes/core/compile/compiled.py
Outdated
@@ -163,6 +169,54 @@ def get_column_type(self, key: str) -> bigframes.dtypes.Dtype: | |||
bigframes.dtypes.ibis_dtype_to_bigframes_dtype(ibis_type), | |||
) | |||
|
|||
def _aggregate_helper( |
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.
Could we get a better name for this method, please. "helper" is very generic, so it's hard to understand what this method is doing.
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.
Renamed it into _aggergate_base
to match the object name BaseIbisIR
.
a66d01b
to
55c9d4f
Compare
0115c73
to
ea3614e
Compare
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
ea3614e
to
5596095
Compare
The end-to-end tests that failed are not caused by this particular change. |
This change introduces the
bigframes.bigquery.array_agg
method forSeriesGroupBy
andDataFrameGroupby
. By default, aggregated arrays are ordered by the underlying sorting columns. Additionally,array_agg
is the inverse operation of(Series|Dataframe).explode()
.Fixes internal bug: 338232748🦕