-
Notifications
You must be signed in to change notification settings - Fork 912
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
Executor for polars logical plans #15504
Executor for polars logical plans #15504
Conversation
0f00a4c
to
b31b7b0
Compare
b31b7b0
to
b10c74d
Compare
c0336b3
to
d86957c
Compare
/ok to test |
11170d7
to
b066d41
Compare
7be8f78
to
76b644c
Compare
78929a1
to
7c2e8e6
Compare
@wence- once you merge in the latest 24.06 I'm happy to take a pass at reviewing. It's fine if you want to keep in draft and you don't think it's quite ready for full review yet, but I figured I'd take a pass once you're reasonably happy so that we can iron out as many issues as possible immediately. |
184ca04
to
0f82d0f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a pass through the tests. I don't have much to say on them content-wise, they all look fine to me and it seems like you have good coverage. Organization-wise, though, I'm confused at what went into test_basic.py
and what got its own module. Can we establish some sort of organizational principle from the get-go?
No good reason for this split (other than |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small comments, but I think this is in more than good enough shape to merge and iterate.
dtype-determination is now simpler.
This removes the need for rename.
de62efe
to
1240b62
Compare
/merge |
Description
This builds out the infrastructure for executing polars logical plans using pylibcudf. See
docs/overview.md
in thecudf_polars
subdirectory for some installation guidance.Deliberately not fully fleshing out packaging and so forth yet.
Test coverage is incomplete but growing. I'd like to get this in so other people can build on top of it.
Checklist