Add libcudf strings split API that accepts regex pattern #10128
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davidwendt
added
feature request
Codecov Report
@@ Coverage Diff @@
## branch-22.04 #10128 +/- ##
================================================
- Coverage 10.42% 10.00% -0.43%
================================================
Files 119 122 +3
Lines 20603 21470 +867
================================================
- Hits 2148 2147 -1
- Misses 18455 19323 +868
Continue to review full report at Codecov.
|
This was referenced Jan 26, 2022
Closed
davidwendt
added
3 - Ready for Review
ttnghia
reviewed
Feb 4, 2022
|
Just add DO_NOT_MERGE label as we are testing it and potentially uncover some issues. I'll report later. |
bdice
reviewed
Feb 7, 2022
nvdbaranec
reviewed
Feb 8, 2022
nvdbaranec
approved these changes
Feb 8, 2022
|
rerun tests |
|
This is ready to merge. It has a sufficient number of reviews and all comments have been addressed. |
We are testing it with some corner cases. @andygrove are you done with the testing? Can this be merged now? |
andygrove
approved these changes
Feb 10, 2022
Yes, this LGTM. I have now approved. |
ttnghia
approved these changes
Feb 10, 2022
ttnghia
reviewed
Feb 10, 2022
|
@gpucibot merge |
rapids-bot bot
pushed a commit
that referenced
this pull request
Feb 14, 2022
This PR adds Java binding for the new strings API `strings::split_re` and `strings::split_record_re`, which allows splitting strings by regular expression delimiters. In addition, the Java string split overloads with default split pattern (an empty string) are removed in this PR. That is because with default empty pattern the Java's split API produces different results than cudf. Finally, some cleanup has been perform automatically thanks to IntelliJ IDE. Depends on #10128. This is breaking change which is fixed by NVIDIA/spark-rapids#4714. Thus, it should be merged at the same time with NVIDIA/spark-rapids#4714. Authors: - Nghia Truong (https://github.com/ttnghia) - David Wendt (https://github.com/davidwendt) Approvers: - Robert (Bobby) Evans (https://github.com/revans2) - Andy Grove (https://github.com/andygrove) URL: #10139
rapids-bot bot
pushed a commit
that referenced
this pull request
Feb 19, 2022
Closes #3584 This depends on libcudf changes in PR #10128 This adds the regex parameter to the cudf strings `split()` function similar to the 1.4.0 Pandas one documented [here](https://pandas.pydata.org/docs/reference/api/pandas.Series.str.split.html). The main difference is that the `pat` parameter will only be interpreted as regex if the `pat` string has more than 1 character and the `regex` parameter is set to `True`. This is to help with consistency and migration from the previous implementation. The 1.3.x Pandas version does not have a `regex` parameter for `split()` but instead will try to interpret the intention of the `pat` parameter without it. This seems a bit dangerous since regex would be much slower for us here. Therefore, the `regex` parameter is required to be set to `True` in the cudf implementation in order to use the regex logic path. Pandas does not support regex for its `rsplit` even though it has been documented and there is an issue here. Authors: - David Wendt (https://github.com/davidwendt) Approvers: - GALI PREM SAGAR (https://github.com/galipremsagar) - Vyas Ramasubramani (https://github.com/vyasr) URL: #10185
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference #3584
This PR adds 4 new libcudf strings APIs for split.
cudf::strings::split_re- split using regex to locate delimiters with table output likecudf::strings::split.cudf::strings::rsplit_re- same assplit_rebut delimiter search starts from the end of each stringcudf::strings::split_record_re- same assplit_rebut returns a list column likesplit_recorddoescudf::strings::rsplit_record_re- same assplit_record_rebut delimiter search starts from the end of each stringLike
split/rsplitthe results try to match Pandas behavior for these. Therecordresults are similar to specifyingexpand=Falsein the Pandassplit/rsplitAPIs. Python/Cython updates for cuDF will be in a follow-on PR.Currently, Pandas does not support regex for its
rspliteven though it has been documented and there is an issue here.New gtests have been added for these along with some additional tests that were missing for the non-regex versions of these APIs.