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

API for JSON unquoted whitespace normalization #15033

Merged
merged 31 commits into from
Mar 4, 2024
Merged

API for JSON unquoted whitespace normalization #15033

merged 31 commits into from
Mar 4, 2024

Conversation

shrshi
Copy link
Contributor

@shrshi shrshi commented Feb 13, 2024

Description

This work is a follow-up to PR #14931 which provided a proof-of-concept for using the a FST to normalize unquoted whitespaces. This PR implements the pre-processing FST in cuIO and adds a JSON reader option that needs to be set to true to invoke the normalizer.
Addresses feature request #14865

Checklist

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

@shrshi shrshi added feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue non-breaking Non-breaking change labels Feb 13, 2024
@shrshi shrshi added the 2 - In Progress Currently a work in progress label Feb 13, 2024
@github-actions github-actions bot added the Java Affects Java cuDF API. label Feb 13, 2024
@shrshi shrshi marked this pull request as ready for review February 13, 2024 22:28
@shrshi shrshi requested review from a team as code owners February 13, 2024 22:28
@shrshi shrshi requested review from harrism, ttnghia and vuule February 13, 2024 22:28
vuule
vuule requested changes Feb 22, 2024
View reviewed changes
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.

Thank you for iterating on the tests.
A few suggestions for the test code:

cpp/tests/io/json_whitespace_normalization_test.cu Show resolved Hide resolved
cpp/tests/io/json_whitespace_normalization_test.cu Outdated Show resolved Hide resolved
cpp/tests/io/json_whitespace_normalization_test.cu Outdated Show resolved Hide resolved
@shrshi shrshi requested a review from vuule February 23, 2024 02:49
vuule
vuule reviewed Feb 23, 2024
View reviewed changes
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.

few nitpicks

cpp/tests/io/json_whitespace_normalization_test.cu Outdated Show resolved Hide resolved
cpp/tests/io/json_whitespace_normalization_test.cu Outdated Show resolved Hide resolved
@shrshi shrshi requested a review from vuule February 27, 2024 01:55
vuule
vuule approved these changes Feb 27, 2024
View reviewed changes
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.

🔥

robertmaynard
robertmaynard approved these changes Feb 27, 2024
View reviewed changes
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

Approving CMake changes

@shrshi
Copy link
Contributor Author

shrshi commented Mar 4, 2024

/merge

@rapids-bot rapids-bot bot merged commit d158ccd into rapidsai:branch-24.04 Mar 4, 2024
73 checks passed
@revans2 revans2 mentioned this pull request Mar 7, 2024
Closed
This was referenced Mar 12, 2024
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vuule vuule vuule approved these changes

@bdice bdice bdice approved these changes

@revans2 revans2 revans2 approved these changes

@robertmaynard robertmaynard robertmaynard approved these changes

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

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

Assignees
No one assigned
Labels
2 - In Progress Currently a work in progress CMake CMake build issue feature request New feature or request Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@shrshi @robertmaynard @revans2 @bdice @vuule