Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add regex flags to strings findall functions #10208

Merged
merged 9 commits into from Feb 8, 2022
Merged

Add regex flags to strings findall functions #10208

merged 9 commits into from Feb 8, 2022

Conversation

davidwendt
Copy link
Contributor

Add the regex_flags parameter to the strings findall() and findall_record functions so that matching regex patterns is consistent other strings regex APIs.

@davidwendt davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. cuDF (Python) Affects Python cuDF API. strings strings issues (C++ and Python) improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 3, 2022
@davidwendt davidwendt self-assigned this Feb 3, 2022
@davidwendt davidwendt added this to PR-WIP in v22.04 Release via automation Feb 3, 2022
@codecov
Copy link

codecov bot commented Feb 3, 2022 •
edited

Codecov Report

Merging #10208 (d148dcd) into branch-22.04 (a7d88cd) will increase coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.04   #10208      +/-   ##
================================================
+ Coverage         10.42%   10.45%   +0.03%     
================================================
  Files               119      122       +3     
  Lines             20603    20530      -73     
================================================
- Hits               2148     2147       -1     
+ Misses            18455    18383      -72
Impacted Files Coverage Δ
python/cudf/cudf/_fuzz_testing/fuzzer.py 0.00% <0.00%> (ø)
python/cudf/cudf/_fuzz_testing/io.py 0.00% <0.00%> (ø)
python/cudf/cudf/_fuzz_testing/main.py 0.00% <0.00%> (ø)
python/cudf/cudf/_version.py 0.00% <0.00%> (ø)
python/cudf/cudf/comm/gpuarrow.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/_base_index.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/datetime.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/methods.py 0.00% <0.00%> (ø)
... and 48 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8af4e84...d148dcd. Read the comment docs.

@davidwendt davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Feb 4, 2022
@davidwendt davidwendt moved this from PR-WIP to PR-Needs review in v22.04 Release Feb 4, 2022
@davidwendt davidwendt marked this pull request as ready for review February 4, 2022 16:22
@davidwendt davidwendt requested review from a team as code owners February 4, 2022 16:22
vyasr
vyasr reviewed Feb 4, 2022
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It occurs to me that findall_record is not actually exposed anywhere in the Python API. Were the bindings just added as a matter of course? That feature was requested for Spark IIRC. I'm wondering about it now because I realized that there were no changes to .py files corresponding to that function.

cpp/include/cudf/strings/findall.hpp Show resolved Hide resolved
cpp/src/strings/search/findall.cu Show resolved Hide resolved
python/cudf/cudf/core/column/string.py Show resolved Hide resolved
v22.04 Release automation moved this from PR-Needs review to PR-Reviewer approved Feb 4, 2022
bdice
bdice approved these changes Feb 4, 2022
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have two requests, but both can (or should) be addressed in follow-up PRs so I am approving. If you want to tackle either of those requests in this PR, I can re-review.

python/cudf/cudf/tests/test_string.py Show resolved Hide resolved
cpp/include/cudf/strings/findall.hpp Show resolved Hide resolved
python/cudf/cudf/_lib/strings/findall.pyx Show resolved Hide resolved
@bdice bdice mentioned this pull request Feb 4, 2022
Closed
@davidwendt
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit bd98bfe into rapidsai:branch-22.04 Feb 8, 2022
v22.04 Release automation moved this from PR-Reviewer approved to Done Feb 8, 2022
@davidwendt davidwendt deleted the add-regex-flags-to-findall branch February 8, 2022 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice approved these changes

@vyasr vyasr vyasr approved these changes

@vuule vuule Awaiting requested review from vuule vuule is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

@nvdbaranec nvdbaranec Awaiting requested review from nvdbaranec nvdbaranec is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@davidwendt davidwendt

Labels
3 - Ready for Review Ready for review by team cuDF (Python) Affects Python cuDF API. improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change strings strings issues (C++ and Python)
Projects
No open projects
v22.04 Release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@davidwendt @vyasr @bdice