-
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
Refactor cudf::strings::count_re API to use count_matches utility #10580
Refactor cudf::strings::count_re API to use count_matches utility #10580
Conversation
Codecov Report
@@ Coverage Diff @@
## branch-22.06 #10580 +/- ##
================================================
+ Coverage 86.31% 86.33% +0.01%
================================================
Files 140 140
Lines 22280 22280
================================================
+ Hits 19232 19236 +4
+ Misses 3048 3044 -4
Continue to review 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.
Nice work! Looks good to me. 👍
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 have a couple of suggestions, but nothing blocking. Feel free to address or not depending on what you think.
@gpucibot merge |
Refactors the
cudf::strings::detail::count_re
function to reuse thecudf::strings::detail::count_matches
utility created for findall, extractall, and split. The kernel code was identical with the only main difference the size of the output column. So the output size was added as a parameter tocount_matches
and the callers appropriately updated.