-
Notifications
You must be signed in to change notification settings - Fork 915
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.
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
Add regex_program
strings replacing java APIs and tests
#12701
Add regex_program
strings replacing java APIs and tests
#12701
Conversation
Signed-off-by: Cindy Jiang <[email protected]>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## branch-23.04 #12701 +/- ##
===============================================
Coverage ? 85.81%
===============================================
Files ? 158
Lines ? 25154
Branches ? 0
===============================================
Hits ? 21587
Misses ? 3567
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit on updating a deprecated method implementation, lgtm.
cudf::jni::native_jstring pattern(env, j_pattern); | ||
auto repl = reinterpret_cast<cudf::string_scalar const *>(j_repl); | ||
return release_as_jlong(cudf::strings::replace_re(scv, pattern.get(), *repl, j_maxrepl)); | ||
auto const column_view = reinterpret_cast<cudf::column_view const *>(j_column_view); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to my comment on other PR, avoid to use column_view
name. This also applies to below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thank you!
Signed-off-by: Cindy Jiang <[email protected]>
@jlowe Thank you! I am not sure I understand the nit. Do you mind elaborating a bit? |
…dyyuanjiang/cudf into replacing-regex-program-apis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I understand the nit. Do you mind elaborating a bit?
My apologies, I apparently didn't fully submit my original suggestion the first time. Should be present on this round of review.
Signed-off-by: Cindy Jiang <[email protected]>
/merge |
Signed-off-by: Cindy Jiang [email protected]
Description
This PR adds replace_re, replace_with_backrefs related
regex_program
java APIs and unit tests.Part of work for NVIDIA/spark-rapids#7295.
Checklist