Expose DataFrame select_exprs method#12520
Conversation
milenkovicm
force-pushed
the
sql_expression
branch
from
5065c9e to
6235c9c
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @milenkovicm
@timsaucer / @Omega359 as you are more familar with DataFrame APIs would you possibly have a chance to review this PR to make sure the API is consistent?
|
This is an interesting addition! I'll take a look at it today. |
|
thanks lads! |
|
The API looks reasonable to me and this is a very nice add. One suggestion on the unit test. |
| /// let ctx = SessionContext::new(); | ||
| /// let df = ctx.read_csv("tests/data/example.csv", CsvReadOptions::new()).await?; | ||
| /// let df = df.select_exprs(&["a * b", "c"])?; | ||
| /// # Ok(()) |
There was a problem hiding this comment.
why do we have commented out parts of example? 🤔
and I found it would be really nice to have an expected answer in example. Like what exactly select_exprs returns
There was a problem hiding this comment.
not sure what do you mean @comphead, I just copied doc from select_columns and adapt it for this interface (description, method name and args)
There was a problem hiding this comment.
alright, its probably what we can do next. If one read the select_expr its great to have a usage but would be nice to see what the method returns
There was a problem hiding this comment.
I get you, will change
comphead
left a comment
comphead
left a comment
There was a problem hiding this comment.
lgtm thanks @milenkovicm
The expected answers we can as a followup
milenkovicm
force-pushed
the
sql_expression
branch
from
6235c9c to
e85a6c6
Compare
|
Thanks everyone |
5 participants
Which issue does this PR close?
Closes #12519.
Rationale for this change
Explained in #12519
What changes are included in this PR?
Are these changes tested?
yes
Are there any user-facing changes?
yes, there is additional public method in
DataFrameNo braking changes