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

[REVIEW] Adding support for explode to cuDF #7140

Merged
merged 10 commits into from
Jan 25, 2021
Merged

[REVIEW] Adding support for explode to cuDF #7140

merged 10 commits into from
Jan 25, 2021

Conversation

hyperbolic2346
Copy link
Contributor

@hyperbolic2346 hyperbolic2346 commented Jan 13, 2021 •
edited
Loading

This is an operation that expands lists into rows and duplicates the existing rows from other columns. Explanation can be found in the issue #6151

partially fixes #6151

Missing pos_explode support required to completely close out #6151

@hyperbolic2346 hyperbolic2346 requested review from a team as code owners January 13, 2021 19:31
@harrism harrism added the libcudf Affects libcudf (C++/CUDA) code. label Jan 13, 2021
@harrism
Copy link
Member

harrism commented Jan 13, 2021

Recommend using a draft PR for WIP, rather than "[WIP]" in the title. This way reviewers don't get notified before you are ready.

@hyperbolic2346 hyperbolic2346 marked this pull request as draft January 13, 2021 21:47
@firestarman firestarman mentioned this pull request Jan 15, 2021
Merged
@hyperbolic2346 hyperbolic2346 changed the title [WIP] Adding support for explode to cuDF [REVIEW] Adding support for explode to cuDF Jan 15, 2021
@hyperbolic2346 hyperbolic2346 marked this pull request as ready for review January 15, 2021 18:58
revans2
revans2 approved these changes Jan 15, 2021
View reviewed changes
Copy link
Contributor

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

Thanks looks good to me

@vuule vuule added the feature request New feature or request label Jan 19, 2021
karthikeyann
karthikeyann requested changes Jan 20, 2021
View reviewed changes
cpp/tests/reshape/explode_tests.cpp Outdated Show resolved Hide resolved
cpp/src/reshape/explode.cu Outdated Show resolved Hide resolved
nvdbaranec
nvdbaranec requested changes Jan 20, 2021
View reviewed changes
Copy link
Contributor

@nvdbaranec nvdbaranec left a comment

Choose a reason for hiding this comment

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

This doesn't handle sliced columns.

@hyperbolic2346
Copy link
Contributor Author

This doesn't handle sliced columns.

I believe this is fixed now.

karthikeyann
karthikeyann approved these changes Jan 21, 2021
View reviewed changes
Copy link
Contributor

@karthikeyann karthikeyann left a comment

Choose a reason for hiding this comment

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

Looks great.

@kkraus14 kkraus14 added 5 - Ready to Merge Testing and reviews complete, ready to merge 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs Review Waiting for reviewer to review or respond 5 - Ready to Merge Testing and reviews complete, ready to merge labels Jan 21, 2021
@kkraus14
Copy link
Collaborator

@nvdbaranec will wait for you to review again before merging.

@kkraus14
Copy link
Collaborator

In a follow up PR we should plumb this into Python as cudf.DataFrame.explode: https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.explode.html

nvdbaranec
nvdbaranec requested changes Jan 21, 2021
View reviewed changes
cpp/src/reshape/explode.cu Outdated Show resolved Hide resolved
cpp/src/reshape/explode.cu Outdated Show resolved Hide resolved
cpp/tests/reshape/explode_tests.cpp Show resolved Hide resolved
@harrism harrism added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 4 - Needs Review Waiting for reviewer to review or respond labels Jan 25, 2021
@harrism
Copy link
Member

harrism commented Jan 25, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit f422391 into rapidsai:branch-0.18 Jan 25, 2021
@harrism harrism mentioned this pull request Jan 25, 2021
Closed
rapids-bot bot pushed a commit that referenced this pull request Jan 25, 2021
@firestarman
Add Java interface for the new API 'explode' (#7151)
bf0c37a 
This PR is to add Java interface for the new API '`explode`', along with its unit tests.

This PR depends on the PR #7140 .

Authors:
  - Liangcai Li (@firestarman)

Approvers:
  - Jason Lowe (@jlowe)
  - Robert (Bobby) Evans (@revans2)

URL: #7151
@hyperbolic2346 hyperbolic2346 deleted the mwilson/explode branch January 25, 2021 19:25
@GaryShen2008 GaryShen2008 mentioned this pull request Feb 10, 2021
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@firestarman firestarman firestarman left review comments

@nvdbaranec nvdbaranec nvdbaranec approved these changes

@jrhemstad jrhemstad jrhemstad approved these changes

@karthikeyann karthikeyann karthikeyann approved these changes

@revans2 revans2 revans2 approved these changes

@vuule vuule Awaiting requested review from vuule

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] memory efficient explode and pos_explode implementations
10 participants
@hyperbolic2346 @harrism @galipremsagar @kkraus14 @revans2 @karthikeyann @firestarman @jrhemstad @nvdbaranec @vuule