Skip to content

Remove host_parse_nested_json. - #15674

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:branch-24.06from
bdice:remove-host_parse_nested_json
May 6, 2024
Merged

Remove host_parse_nested_json.#15674
rapids-bot[bot] merged 1 commit into
NVIDIA:branch-24.06from
bdice:remove-host_parse_nested_json

Conversation

@bdice

@bdice bdice commented May 6, 2024

Copy link
Copy Markdown
Contributor

Description

This PR addresses a task from #15537 to remove the host_parse_nested_json code path and corresponding tests. See discussion in #15568 (comment).

Checklist

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

@github-actions github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label May 6, 2024
@bdice bdice added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 6, 2024
@bdice bdice mentioned this pull request May 6, 2024
7 tasks
@bdice
bdice marked this pull request as ready for review May 6, 2024 20:44
@bdice
bdice requested a review from a team as a code owner May 6, 2024 20:44
@bdice
bdice requested review from davidwendt and harrism May 6, 2024 20:44
@bdice

bdice commented May 6, 2024

Copy link
Copy Markdown
Contributor Author

Although this is a deletion, I think this is "non-breaking" because the changes are all in the src directory, and are not public.

* @param mr Optional, resource with which to allocate
* @return The data parsed from the given JSON input
*/
table_with_metadata device_parse_nested_json(device_span<SymbolT const> input,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we rename device_parse_nested_json to parse_nested_json? We are removing the host implementation in this PR, which previously existed only for debugging.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm ok with keeping the name.

@davidwendt davidwendt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Thanks for doing this.

@harrism harrism left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good job!

@bdice

bdice commented May 6, 2024

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit dcd0d6b into NVIDIA:branch-24.06 May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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