chore: simplify parsing expressions in group-by

[MarcoGorelli]

This had been annoying me for a while

We're using CompliantNamespace.parse_into_expr, which accepts Python scalars, in {DataFrame,LazyFrame}.{select,with_columns}, even though there we don't accept Python scalars

I think this simplifies things a little, and means we can do .alias within flatten_and_extract before going to the compliant level (which I think is not a bad thing, given that there we're still at the Narwhals level. this will also help in some follow-up / skunkworks)

[dangotbanned]

Gotta say this has me pretty confused

This had been annoying me for a while

We're using CompliantNamespace.parse_into_expr,

I only added this 1 week ago and you approved it?

• refactor: Split up BaseFrame._extract_compliant #3054

But this doesn't just revert that PR, it also goes in the opposite direction of what you said you wanted in

• Refine public Compliant* methods #2713

The purpose of CompliantNamespace.parse_into_expr was to stop calling so many private methods from the narwhals-level

Now, those methods both don't exist at the compliant-level and they're being called as if they did
So this seems like the worst of both worlds to me 🤔

[MarcoGorelli]

Gotta say this has me pretty confused

This had been annoying me for a while

sorry the "for a while" was referring to the group-by logic not being able to re-use flatten_and_extract and repeating the same logic from select / with_columns

will iterate on this

[MarcoGorelli]

Have corrected - it should now not be using anything non-compliant, right?

[dangotbanned]

Have corrected - it should now not be using anything non-compliant, right?

I really would've liked a chance to review this.

CompliantExpr._from_series?

I've been waiting for reviews on PRs that have approval, but just not yours yet.

Rushing this through after I've objected (#3106 (comment)) to parts of it - and not letting me respond - makes me feel like my time isn't valued.
That stings especially hard, because the original PR only exists due to you asking for me to start that work (#2713 (comment))

[MarcoGorelli]

CompliantExpr._from_series?

I'd say that it's EagerExpr._from_series which is being used, and only within EagerSeries? My understanding is that we're OK with those, as they represent a way to share code between the eager implementations

It's the purely Compliant classes we want to keep lean and stable

• CompliantSeries
• CompliantExpr
• CompliantDataFrame
• CompliantLazyFrame
• CompliantNamespace

I'm really sorry to have hurt you here, and thank you for having told me, will make sure to wait in the future

[MarcoGorelli]

reverted and re-opened in #3110

[dangotbanned]

I'm really sorry to have hurt you here, and thank you for having told me, will make sure to wait in the future

Thanks @MarcoGorelli, that really means a lot ❤️