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.

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

Deprecate libcudf regex APIs accepting pattern strings directly #12891

Merged

Conversation

davidwendt
Copy link
Contributor

Description

Deprecates the libcudf regex APIs that do not accept cudf::strings::regex_program objects.
The libcudf regex functions were converted to accept regex_program objects instead of a pattern string and regex-flags directly in PR #11927
Most of this is removing calls to the deprecated functions in the gtests.
The declarations in the headers had to be reordered to make the reference links work in the doxygen output.

These deprecated functions will be removed in a future release.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. tech debt improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 7, 2023
@davidwendt davidwendt self-assigned this Mar 7, 2023
@davidwendt davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Mar 7, 2023
@davidwendt davidwendt marked this pull request as ready for review March 7, 2023 23:55
@davidwendt davidwendt requested a review from a team as a code owner March 7, 2023 23:55
@davidwendt davidwendt requested review from ttnghia and vuule March 7, 2023 23:55
@davidwendt
Copy link
Contributor Author

Apologize for reordering the declarations in the headers to make doxygen work. They headers look difficult to review now. Essentially there are two APIs one with string_view pattern parameter and one with regex_program parameter. The order of these were swapped in the header file at each location so the regex_program one would appear before the string_view one.

@vuule
Copy link
Contributor

vuule commented Mar 8, 2023

Do you have a timeline to remove the deprecated APIs?
I'm wondering if we should add deprecation warning message(s) once logger is in place, so deprecation is more visible.

@davidwendt
Copy link
Contributor Author

Do you have a timeline to remove the deprecated APIs?

I was planning to remove them in 23.06.

I'm wondering if we should add deprecation warning message(s) once logger is in place, so deprecation is more visible.

The compiler will give a deprecation warning. I'm not sure we should warn actual end users.

@vuule
Copy link
Contributor

vuule commented Mar 8, 2023

Oh, right, [[deprecated]]. Yeah, that sounds perfect.

Copy link
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

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

doxygen really spiced this one up :D

@davidwendt
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 9299c2b into rapidsai:branch-23.04 Mar 9, 2023
@davidwendt davidwendt deleted the dep-regex-pattern-str-apis branch March 9, 2023 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants