Skip to content
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

add support for pl$concat(<LazyFrame>, . . . ) + add to_supertypes auto casting #407

Merged
merged 18 commits into from
Oct 9, 2023

Conversation

sorhawell
Copy link
Collaborator

@sorhawell sorhawell commented Sep 30, 2023

closes #386

extras:

allow syntax pl$concat(. . .) and not only pl$concat(list(. . .)), this is matches behavoir for pl$DataFrame and $select().
to facility unit testing also implemented <DataFrame>$n_chunks() to count chunks
add robj_to! DataFrame and LazyFrame
non-breaking upgrades to internal functions unpack_list() [skip unpack if these classes] + get_err_ctx() [pick only this context type] + Plain_err() [allow multiline txt input]

minor deviation from py-polars

This PR deviate slightly from py-polars pl.concat which is a quite verbose implementation:
how = "align" is not implemented (it could be but it is just another task.)
how = "xyz_releaxed" are just accessed directly via arg to_supertypes which will cast differing types to any shared supertype.
r-polars do support to_supertypes also for concat Series, py-polars has not implemented that. Maybe that is why they don't just expose to_supertypes.

to_supertypes = TRUE is like R where c(1, "b") yields [1] "1" "b" and to_supertypes = FALSE will throw errors if not exactly the same type.

@sorhawell sorhawell marked this pull request as ready for review October 1, 2023 21:37
@sorhawell sorhawell changed the title WIP: Lazy concat add support for pl$concat(<LazyFrame>, . . . ) + add to_supertypes auto casting Oct 1, 2023
Copy link
Collaborator

@etiennebacher etiennebacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the Rust side looks good but I have several comments on the R side because for now this doesn't match entirely the behavior of py-polars

R/functions__eager.R Outdated Show resolved Hide resolved
R/functions__eager.R Show resolved Hide resolved
R/functions__eager.R Outdated Show resolved Hide resolved
R/functions__eager.R Outdated Show resolved Hide resolved
R/functions__eager.R Outdated Show resolved Hide resolved
R/functions__eager.R Outdated Show resolved Hide resolved
R/functions__eager.R Show resolved Hide resolved
R/functions__eager.R Show resolved Hide resolved
R/functions__eager.R Outdated Show resolved Hide resolved
R/dataframe__frame.R Outdated Show resolved Hide resolved
@sorhawell
Copy link
Collaborator Author

@etiennebacher I think we have addressed all first round of comments.

@etiennebacher
Copy link
Collaborator

Thanks, I think this should be good but I'll just do some tweaks in docs and fix typos

@etiennebacher etiennebacher merged commit ec81870 into main Oct 9, 2023
11 checks passed
@etiennebacher etiennebacher deleted the lazy_concat branch October 9, 2023 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

concat() should work with LazyFrames
2 participants