You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/10/07 12:03:16 UTC

[GitHub] [arrow] maartenbreddels commented on pull request #8271: ARROW-9991: [C++] split kernels for strings/binary

maartenbreddels commented on pull request #8271:
URL: https://github.com/apache/arrow/pull/8271#issuecomment-704888455


   @pitrou @jorisvandenbossche I think this is ready for review, provided the points below are not an issue.
   
   Open questions
    * kernel name: string_split_pattern vs split_pattern. I can imagine that we also want to support binary_split_pattern, so should we have to separate kernel names for this, or keep it split_pattern and implement support for binary in the future?
    * API docs: I'm not sure the compute.rst docs I added should follow some standard, or if that table can be produce using some tool, I've done it by hand for now.
    * Should split_pattern be a binary kernel, where pattern is the second argument? As @jorisvandenbossche  suggested we could start by only implementing only the second argument being a scalar. I tried to implement this, but I had to change the testing framework/tools for this a lot. So I think the should be a separate issue, otherwise this PR will never end. OTOH, this will mean a breaking change.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org