Skip to content

Conversation

@fexolm
Copy link
Contributor

@fexolm fexolm commented Nov 21, 2019

As discussed in ARROW-7085, I've implemented one of the ways to provide custom column builder to a column.
However, it doesn't look like final solution for me. Hope to hear your thought about possible implementation.

@github-actions
Copy link

@fexolm
Copy link
Contributor Author

fexolm commented Nov 25, 2019

@pitrou @wesm PTAL

@pitrou
Copy link
Member

pitrou commented Nov 25, 2019

@fexolm The Parser, ColumnBuilder.. APIs are currently internal, and slightly messy. We'll have to think about cleaning them up before making them public.

@fexolm
Copy link
Contributor Author

fexolm commented Nov 25, 2019

@pitrou the other option is to define a class inherited from ExtentionType with an additional method for creating column builder for that type.

This solution wouldn't require changing stable API.
I can make a sample implementation if it sounds better for you.

@pitrou
Copy link
Member

pitrou commented Nov 25, 2019

No, this would have the same problem since ColumnBuilder takes a BlockParser.

@anton-malakhov
Copy link

anton-malakhov commented Nov 25, 2019

@pitrou We would not mind if this ExtensionType based approach is implemented in internal namespace as well, so it will not appear as a stable API right away.
[updated] I mean ExtensionType is already defined. What we need is additional flexibility in of its usage which will not touch public API but will allow to define our custom converter even if it is internal or uses internal inputs at the moment,

@wesm
Copy link
Member

wesm commented May 24, 2020

Closing this PR since it has grown stale. I think it would be worth defining a public API for this, but we're going to have to take time to propose clean, well-documented interfaces rather than poking through internal APIs into the public API without any changes

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.

4 participants