-
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
Remove host_parse_nested_json. #15674
Remove host_parse_nested_json. #15674
Conversation
Although this is a deletion, I think this is "non-breaking" because the changes are all in the |
* @param options Parsing options specifying the parsing behaviour | ||
* @param stream The CUDA stream to which kernels are dispatched | ||
* @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, |
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.
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.
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'm ok with keeping the name.
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.
Looks good. Thanks for doing this.
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.
Good job!
/merge |
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