Add GroupBy.aggregate (and tpch-1 query to examples)

[MarcoGorelli]

closes #274

the gist of the PR is that it lets you write

    result = (
        lineitem.filter(mask)
        .group_by(["l_returnflag", "l_linestatus"])
        .aggregate(
            namespace.Aggregation.sum("l_quantity").rename("sum_qty"),
            namespace.Aggregation.sum("l_extendedprice").rename("sum_base_price"),
            namespace.Aggregation.sum("l_disc_price").rename("sum_disc_price"),
            namespace.Aggregation.sum("change").rename("sum_charge"),
            namespace.Aggregation.mean("l_quantity").rename("avg_qty"),
            namespace.Aggregation.mean("l_discount").rename("avg_disc"),
            namespace.Aggregation.size().rename("count_order"),
        )
        .sort(["l_returnflag", "l_linestatus"])
    )

[MarcoGorelli]

this syntax though...

[MarcoGorelli]

Hi @shwina - are you ok with this syntax?

Personally I think it's worse than what's in any existing dataframe library, and I can't imagine any user ever wanting to write code like this

but maybe it's just me

[shwina]

Please correct me if I'm wrong, but I thought the goal of the standard right now is to provide an API focused on third-party library developers (not end users). This is why we have been comfortable sacrificing syntactic crispness or an expressive API in favor of being the "lowest common denominator" that all libraries can implement.

I think this necessarily means the API isn't quite as nice to work with for the end-user.

For example, changing get_column_by_name to just [ ] in the code above would be a massive boost in readability, but we explicitly decided against it because (IIRC) we wanted library authors to have the freedom to decide what [ ] should mean for their library

[shwina]

That being said, I agree with you 100% that this looks a mess. It's a question whether library developers are going to be OK with dealing with a messy API to get cross-library compatibility in return...

[MarcoGorelli]

I agree with you 100% that this looks a mess

Well I'm glad we could find some common ground 😄

Let's discuss more next week - I'm genuinely interested in finding a solution that works for everybody

My current prediction is that, unless the standard drastically improves, that libraries will just support pandas and Polars and ignore the standard completely

The end result for cudf will be that you'll be no better off than you are now

[MarcoGorelli]

As I was saying...(emphasis mine)

I'm pretty upset about having to use df.get_column_by_name("a") instead of a simpler df["a"] or col("a"). This will obfuscate our code and impair readability, and therefore we may consider keeping our duplicate logic

#287

[shwina]

That's fair. We should shorten the name.

[MarcoGorelli]

any suggestions?

[kkraus14]

Being addressed in #290

[kkraus14]

LGTM other than the move to df.col

[MarcoGorelli]

thanks for your review

we can rename to col if/when the other PR is in