Add separator-on-null parameter to strings concatenate APIs - #8282
Merged
rapids-bot[bot] merged 10 commits intoMay 24, 2021
Conversation
ttnghia
requested changes
May 19, 2021
tgravescs
reviewed
May 19, 2021
Codecov Report
@@ Coverage Diff @@
## branch-21.06 #8282 +/- ##
===============================================
Coverage ? 82.88%
===============================================
Files ? 105
Lines ? 17874
Branches ? 0
===============================================
Hits ? 14814
Misses ? 3060
Partials ? 0 Continue to review full report at Codecov.
|
davidwendt
marked this pull request as ready for review
May 19, 2021 17:40
robertmaynard
approved these changes
May 19, 2021
robertmaynard
left a comment
Contributor
There was a problem hiding this comment.
CMake changes look good to me
ttnghia
approved these changes
May 19, 2021
cwharris
reviewed
May 20, 2021
tgravescs
approved these changes
May 21, 2021
tgravescs
left a comment
Contributor
There was a problem hiding this comment.
tested this latest patch out (along with pr 8285) and all my Spark tests pass so we have the desired null handling behavior.
cwharris
approved these changes
May 21, 2021
Contributor
Author
|
@gpucibot merge |
rapids-bot Bot
pushed a commit
that referenced
this pull request
May 24, 2021
to stringConcatenate when using a scalar separator. Reference #8282 changed to throw an exception if only a single column is passed in to the stringConcatenate using scalar separator. Update our Java test for that functionality. Signed-off-by: Thomas Graves <tgraves@nvidia.com> Authors: - Thomas Graves (https://github.com/tgravescs) Approvers: - Robert (Bobby) Evans (https://github.com/revans2) - Jason Lowe (https://github.com/jlowe) URL: #8330
rapids-bot Bot
pushed a commit
that referenced
this pull request
May 26, 2021
This PR implements a new option for `strings::join_list_elements` on top of #8282. In particular, the new option is: ``` /** * @brief Setting for specifying what will be output from `join_list_elements` when an input list * is empty. */ enum class output_if_empty_list { EMPTY_STRING, ///< Empty list will result in empty string NULL_ELEMENT ///< Empty list will result in a null }; ``` This new option is necessary for implementing `concat_ws` in Spark, since the behavior of the output string is required to be different depending on the situation. Currently blocked from merging by #8282. Authors: - Nghia Truong (https://github.com/ttnghia) - David Wendt (https://github.com/davidwendt) Approvers: - Robert Maynard (https://github.com/robertmaynard) - Keith Kraus (https://github.com/kkraus14) - Mike Wilson (https://github.com/hyperbolic2346) - David Wendt (https://github.com/davidwendt) - GALI PREM SAGAR (https://github.com/galipremsagar) - Ashwin Srinath (https://github.com/shwina) URL: #8285
rapids-bot Bot
pushed a commit
that referenced
this pull request
May 26, 2021
Add java api's to be able to call string concatenate with separators. DO NOT MERGE until: #8285 and #8282 are merged. We need those changes for the API used in this PR as well as functionality to match what Spark needs. new arguments were added to the existing concatenate api that takes a scalar for the separator, so I extended that. I added new api's for the concatenate api that takes a column as the separator. I also added new api's for both join_list_elements api's, one with scalar separator and one with column separator. Authors: - Thomas Graves (https://github.com/tgravescs) Approvers: - Robert (Bobby) Evans (https://github.com/revans2) - Jason Lowe (https://github.com/jlowe) URL: #8289
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4728
This PR adds a new parameter to the
cudf::strings::concatenateAPIs to specify if separators should be added between null entries when the null-replacement (narep) parameter is valid. If the narep scalar is invalid (i.e. null itself) then the entire output row becomes null. If not, separators are added between each element. Examples:The new parameter is an enum
separator_on_nullswhich hasYESorNOsettings. The default parameter value will beYESto keep the current behavior as expected by Python cudf and for consistency with Pandas behavior.Specifying
NOhere will suppress the separator with null elements (when narep is valid).This PR also changes the name of the
cudf::strings::concatenate_list_elementsAPI tocudf::strings::join_list_elementsinstead. The API pattern and behavior more mimic thecudf::strings::join_stringsthen the concatenate functions. Also, these are called by the Pythonjoinfunctions so the rename makes it more consistent with cudf.This is a breaking change in order to make these APIs more consistent. Previously, the separators column version was returning nulls only for an all-null row. This has been changed to honor the
separator_on_nullparameter instead. Currently there was no Python cudf API calling this version. Only the rename required minor changes to the Cython layer.The gtests were updated to reflect the new behavior. None of the pytests required any changes since the default parameter value matches the original behavior for those APIs that cudf actually calls.