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.

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

Allow merging index column with data column using keyword "on" #7736

Merged
merged 190 commits into from
Apr 2, 2021

Conversation

skirui-source
Copy link
Contributor

fixes #5014
replaces PR #7569

@github-actions github-actions bot removed the libcudf Affects libcudf (C++/CUDA) code. label Mar 30, 2021
@kkraus14 kkraus14 removed request for a team and kkraus14 March 30, 2021 18:57
@skirui-source skirui-source added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Mar 30, 2021
@skirui-source skirui-source requested a review from kkraus14 April 1, 2021 03:59
Copy link
Contributor

@isVoid isVoid left a comment

Choose a reason for hiding this comment

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

This feature lacks test cases.

python/cudf/cudf/core/join/join.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/join/join.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/join/join.py Outdated Show resolved Hide resolved
@kkraus14
Copy link
Collaborator

kkraus14 commented Apr 1, 2021

This needs tests as well

Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

Nice work! I suggested some improvements.

python/cudf/cudf/core/join/join.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/join/join.py Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_joining.py Outdated Show resolved Hide resolved
Copy link
Contributor

@isVoid isVoid left a comment

Choose a reason for hiding this comment

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

lgtm cc @kkraus14 for a second look

@vyasr vyasr added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Apr 2, 2021
@kkraus14
Copy link
Collaborator

kkraus14 commented Apr 2, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 1825001 into rapidsai:branch-0.19 Apr 2, 2021
@skirui-source skirui-source deleted the mergeindexondata branch May 6, 2021 17:43
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 breaking Breaking change feature request New feature or request Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Allow merging an index column with data column using keyword "on"
5 participants