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.

Sign up for GitHub

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

Jump to bottom

ARROW-13855: [C++][Python] Implement C data interface support for extension types #11071

Closed
wants to merge 1 commit into from
Closed

ARROW-13855: [C++][Python] Implement C data interface support for extension types #11071

wants to merge 1 commit into from

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Sep 2, 2021

No description provided.

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

https://issues.apache.org/jira/browse/ARROW-13855

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@pitrou pitrou requested a review from lidavidm September 2, 2021 18:05
@jorgecarleitao jorgecarleitao mentioned this pull request Sep 2, 2021
Merged
@jorgecarleitao
Copy link
Member

jorgecarleitao commented Sep 2, 2021 •
edited
Loading

Can't review the c++ in depth, but from what I can read, I took an equivalent approach (jorgecarleitao/arrow2#363), which I am glad of.

To import

  1. read metadata
  2. detect extension from meta
  3. use extension

To export

  1. create meta map with extension name and meta
  2. fill meta map with remaining meta from field
  3. unwrap inner from extension
  4. expose in c data interface

There is a small divergence in that we remove the extension fields from the field's metadata when consuming from IPC and FFI, and automatically convert them to the DataType::Extension variant (and vice-versa on export). AFAI understand this is an implementation detail that does not impact how they communicate.

@pitrou
Copy link
Member Author

pitrou commented Sep 2, 2021

Ah, we could remove the metadata fields here too (I think we do it in the IPC reader).

lidavidm
lidavidm approved these changes Sep 2, 2021
View reviewed changes
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me.

@lidavidm lidavidm closed this in 5ead375 Sep 3, 2021
@pitrou pitrou deleted the ARROW-13855-export-extension branch September 3, 2021 15:47
zeroshade pushed a commit to zeroshade/arrow that referenced this pull request Sep 12, 2021
@pitrou
ARROW-13855: [C++][Python] Implement C data interface support for ext…
661aff0 
…ension types

Closes apache#11071 from pitrou/ARROW-13855-export-extension

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: David Li <[email protected]>
ViniciusSouzaRoque pushed a commit to s1mbi0se/arrow that referenced this pull request Oct 20, 2021
@pitrou @ViniciusSouzaRoque
ARROW-13855: [C++][Python] Implement C data interface support for ext…
31a39a7 
…ension types

Closes apache#11071 from pitrou/ARROW-13855-export-extension

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: David Li <[email protected]>
@asfimport asfimport mentioned this pull request Sep 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidavidm lidavidm lidavidm approved these changes

Assignees
No one assigned
Labels
Component: C++ Component: Python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pitrou @jorgecarleitao @lidavidm