Skip to content

Support create lists column from a list_scalar - #8185

Merged
rapids-bot[bot] merged 20 commits into
NVIDIA:branch-21.06from
isVoid:list_column_from_scalar
May 20, 2021
Merged

Support create lists column from a list_scalar#8185
rapids-bot[bot] merged 20 commits into
NVIDIA:branch-21.06from
isVoid:list_column_from_scalar

Conversation

@isVoid

@isVoid isVoid commented May 7, 2021

Copy link
Copy Markdown
Contributor

This PR adds support to make_column_from_scalar for list_scalar. For 0-length columns, a well-formed LIST type column, whose child column has the same column hierarchy to the row data stored in list_scalar is returned.

Example:

slr.data = [1, 2, 3] // An integer list of 1, 2, 3, `data` is an INT column
make_column_from_scalar(s, 2) // List<int> column: {[1, 2, 3], [1, 2, 3]}, whose child column is an `INT` column.

slr.data = [[1, 2], [3]] // A list of integer lists, `data` is a List<int> column
make_column_from_scalar(s, 0) // Well formed, 0-length List<List<int>> column, whose child column is a List<int> column.

Closes #8088

@github-actions github-actions Bot added CMake CMake build issue conda libcudf Affects libcudf (C++/CUDA) code. labels May 7, 2021
@isVoid isVoid self-assigned this May 7, 2021
@codecov

codecov Bot commented May 7, 2021

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.06@8834ed6). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.06    #8185   +/-   ##
===============================================
  Coverage                ?   82.84%           
===============================================
  Files                   ?      105           
  Lines                   ?    17863           
  Branches                ?        0           
===============================================
  Hits                    ?    14798           
  Misses                  ?     3065           
  Partials                ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8834ed6...23cb574. Read the comment docs.

@github-actions github-actions Bot removed the CMake CMake build issue label May 11, 2021
@isVoid
isVoid marked this pull request as ready for review May 11, 2021 04:11
@isVoid
isVoid requested review from a team as code owners May 11, 2021 04:11
@isVoid isVoid added feature request New feature or request 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels May 11, 2021

@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.

I think stream and MR need to be used explicitly. Detail APIs should call detail APIs.

Comment thread cpp/src/lists/lists_column_factories.cu Outdated
Comment thread cpp/src/lists/lists_column_factories.cu Outdated

@devavret devavret 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.

You can check in the beginning whether the size == 1 and make a mr that's either the default or the one passed in the arguments. Then you can use the same mr thoughout and avoid duplicating code.

Comment thread cpp/src/lists/lists_column_factories.cu Outdated
@isVoid
isVoid requested review from devavret and harrism May 12, 2021 23:43

@ajschmidt8 ajschmidt8 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

Comment thread cpp/src/lists/lists_column_factories.cu Outdated
@isVoid isVoid added 2 - In Progress Currently a work in progress and removed 3 - Ready for Review Ready for review by team labels May 13, 2021
@isVoid isVoid added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels May 13, 2021
@ajschmidt8 ajschmidt8 changed the title Support create lists column from a list_scalar Support create lists column from a list_scalar May 18, 2021
@ajschmidt8 ajschmidt8 changed the title Support create lists column from a list_scalar Support create lists column from a list_scalar May 18, 2021
@nvdbaranec

Copy link
Copy Markdown
Contributor

This PR ready to merge?

@isVoid

isVoid commented May 19, 2021

Copy link
Copy Markdown
Contributor Author

Yup, I think it's good to go.

@harrism

harrism commented May 20, 2021

Copy link
Copy Markdown
Contributor

@isVoid you can merge with @gpucibot merge. And if you don't want to merge yourself, please use labels to indicate ready to merge.

@harrism harrism added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels May 20, 2021
@harrism

harrism commented May 20, 2021

Copy link
Copy Markdown
Contributor

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit c732cef into NVIDIA:branch-21.06 May 20, 2021
rapids-bot Bot pushed a commit that referenced this pull request May 24, 2021
This small PR is to replace the JNI implementation with the corresponding cudf API `make_column_from_scalar`.

The PR #8185 has added the support for nested type, so it is ok to do this now.

Signed-off-by: Firestarman <firestarmanllc@gmail.com>

Authors:
  - Liangcai Li (https://github.com/firestarman)

Approvers:
  - Bobby Wang (https://github.com/wbo4958)
  - Robert (Bobby) Evans (https://github.com/revans2)
  - Jason Lowe (https://github.com/jlowe)

URL: #8310
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to Merge Testing and reviews complete, ready to merge feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEA] Let cudf::make_column_from_scalar support list type

5 participants