Skip to content

ARROW-12944: [C++] String capitalize kernel - #10857

Closed
edponce wants to merge 8 commits into
apache:masterfrom
edponce:ARROW-12944-String-capitalize-kernel
Closed

ARROW-12944: [C++] String capitalize kernel#10857
edponce wants to merge 8 commits into
apache:masterfrom
edponce:ARROW-12944-String-capitalize-kernel

Conversation

@edponce

Copy link
Copy Markdown
Contributor

This PR adds scalar compute functions for string capitalization, namely "ascii_capitalize" and "utf8_capitalize".

@github-actions

Copy link
Copy Markdown

Comment threadcpp/src/arrow/compute/kernels/scalar_string_test.cc Outdated
Comment threadcpp/src/arrow/compute/kernels/scalar_string.cc Outdated
Comment threadcpp/src/arrow/compute/kernels/scalar_string.cc Outdated
@edponce
edponceforce-pushed the ARROW-12944-String-capitalize-kernel branch from c1b73ae to 6ae88fdCompareAugust 3, 2021 14:48
@edponce
edponce requested a review from pitrouAugust 3, 2021 15:15
Comment threadcpp/src/arrow/compute/kernels/scalar_string.cc Outdated
@pitrou
pitrouforce-pushed the ARROW-12944-String-capitalize-kernel branch from 18c4f01 to 9d7f298CompareAugust 4, 2021 12:12
@pitrou

Copy link
Copy Markdown
Member

Rebased, will merge if green.

@edponce
edponceforce-pushed the ARROW-12944-String-capitalize-kernel branch from 9d7f298 to 4f82d0dCompareAugust 4, 2021 12:36
@edponce

Copy link
Copy Markdown
ContributorAuthor

@pitrou I pushed a change after you rebased, so I will fix conflicts and re-push.

@pitrou

Copy link
Copy Markdown
Member

Hmm, another commit was lost then...

@pitrou

Copy link
Copy Markdown
Member

Recommend starting from 9d7f298 and then applying the buffer overflow fix.

@edponce
edponceforce-pushed the ARROW-12944-String-capitalize-kernel branch from 4f82d0d to 997919eCompareAugust 4, 2021 13:41
@pitroupitrou closed this in 99c9231Aug 4, 2021
@edponce

Copy link
Copy Markdown
ContributorAuthor

Some tests invoke the incorrect kernel (ASCII test uses utf8_capitalize and viceversa). These "typos" are fixed in #10869

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@edponce@pitrou