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

Defer to C++ equality and hashing for pylibcudf DataType and Aggregation objects #15732

Merged
merged 2 commits into from
May 16, 2024
Merged

Defer to C++ equality and hashing for pylibcudf DataType and Aggregation objects #15732

merged 2 commits into from
May 16, 2024

Conversation

wence-
Copy link
Contributor

@wence- wence- commented May 13, 2024

Description

Since the C++ layer provides implementations of these, use them, rather than redoing an implementation. This avoids things ever getting out of sync.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@wence- wence- requested a review from a team as a code owner May 13, 2024 15:34
@wence- wence- requested review from bdice and brandon-b-miller May 13, 2024 15:34
@github-actions github-actions bot added the Python Affects Python cuDF API. label May 13, 2024
@wence- wence- added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed Python Affects Python cuDF API. labels May 13, 2024
wence- added 2 commits May 15, 2024 17:12
@wence-
OAOO __eq__ on DataType
ab1b2b9 
The C++ data_type type implements operator==, so we should use it in
Cython to avoid the definitions ever going out of sync.
@wence-
Implement __eq__ and __hash__ for Aggregation objects
f67922a 
Use the matching C++ implementations for OAOO.
@wence- wence- force-pushed the wence/fea/pylibcudf-oaoo branch from c0089bb to f67922a Compare May 15, 2024 17:13
@github-actions github-actions bot added the Python Affects Python cuDF API. label May 15, 2024
vyasr
vyasr approved these changes May 15, 2024
View reviewed changes
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.

LGTM, thanks!

@wence-
Copy link
Contributor Author

wence- commented May 16, 2024

/merge

@rapids-bot rapids-bot bot merged commit 0a544c2 into rapidsai:branch-24.06 May 16, 2024
70 checks passed
@wence- wence- deleted the wence/fea/pylibcudf-oaoo branch May 16, 2024 09:16
@vyasr vyasr added the pylibcudf Issues specific to the pylibcudf package label May 28, 2024
@wence- wence- mentioned this pull request Jun 4, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vyasr vyasr vyasr approved these changes

@bdice bdice Awaiting requested review from bdice bdice is a code owner automatically assigned from rapidsai/cudf-python-codeowners

@brandon-b-miller brandon-b-miller Awaiting requested review from brandon-b-miller brandon-b-miller is a code owner automatically assigned from rapidsai/cudf-python-codeowners

Assignees
No one assigned
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change pylibcudf Issues specific to the pylibcudf package Python Affects Python cuDF API.
Projects
cuDF Python
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wence- @vyasr