-
Notifications
You must be signed in to change notification settings - Fork 601
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
feat(pyarrow): support arrow PyCapsule interface in more places #9663
Conversation
I'm going to split the schema one out into a new PR, I'm not sure if the |
I'm not sure if supporting Indeed, this change originally ran into test failures because supporting this implicitly added support for I'm tempted to do the same here for all objects exposing Thoughts @kylebarron? |
Thinking more on this, I think this is the correct approach. I really don't want to be encouraging users into doing inefficient things, and implicitly slurping up data in anything exposing a |
Managing when to materialize an Arrow stream is indeed tricky. I'd say it's best to be a conscious decision by the end user, where they can choose among APIs that do or don't materialize the stream. E.g. this is why both In terms of ibis, is it possible for something like a As long as it's documented that an |
I think this is fair. As I understand it: users can explicitly pass data with |
Not really, no. Ibis expects that data in an TL;DR: Re: your other comments, I'm having a hard time understanding if you're agreeing or disagreeing with my proposal above :). To clarify, there are two options here:
Right now I'm leaning towards option 2, but can see both sides. Note that if we do decide that option 1 is what we want, this would require a larger lift since currently |
If it's well-documented that You had previously referenced con.create_table("new", record_batch_reader) Does that implicitly use an con.create_table("new", ibis.memtable(record_batch_reader))
It happens that |
28aff8a
to
10bac0f
Compare
10bac0f
to
e48c90f
Compare
Two changes here:Support__arrow_c_schema__
inibis.Schema
objects__arrow_c_stream__
as inputs toibis.memtable
Fixes #9660.