Expunge NamedColumn#16962
Conversation
2b9b14f to
5543a8f
Compare
Everything in the expression evaluation now operates on columns without names. DataFrame construction takes either a mapping from string-valued names to columns, or a sequence of pairs of names and columns. This removes some duplicate code in the NamedColumn class (by removing it) where we had to fight the inheritance hierarchy. - Closes rapidsai#16272
5543a8f to
7ef586b
Compare
There was a problem hiding this comment.
Why not keep NamedColumn, but with a "has-a" relationship with Column as you mentioned in the original issue instead of removing it?
Good question. I tried that first, but then throughout the IR evaluation, since everything is a But perhaps it is easier if |
7ef586b to
3708d95
Compare
Rather than separating names from columns with tuple return values in IR evaluation, allow columns to optionally have names that are copied around. When we construct a dataframe from columns we now must assert that the column has a name, but otherwise this is OK.
3708d95 to
1a45018
Compare
OK, I had a go at just letting If we prefer this approach, I'm happy to go with it, otherwise we can go back to having a full name/Column split. |
I'm +1 to this approach @wence-, it feels cleaner to me. |
vyasr
left a comment
There was a problem hiding this comment.
Some small comments, but generally LGTM.
Matt711
left a comment
There was a problem hiding this comment.
Thanks! LGTM, just a docstring fix
|
/merge |
Description
Everything in the expression evaluation now operates on columns without names. DataFrame construction takes either a mapping from string-valued names to columns, or a sequence of pairs of names and columns.
This removes some duplicate code in the NamedColumn class (by removing it) where we had to fight the inheritance hierarchy.
Checklist