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

Fix dask token normalization #14829

Merged
merged 10 commits into from
Jan 31, 2024
Merged

Fix dask token normalization #14829

merged 10 commits into from
Jan 31, 2024

Conversation

rjzamora
Copy link
Member

Description

This PR fixes cudf's __dask_tokenization__ definitions so that they will produce data that can be deterministically tokenized when a MultiIndex is present. I ran into this problem in dask-expr for an index with datetime data (a case reflected by the new test).

Checklist

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

@rjzamora rjzamora added bug Something isn't working 2 - In Progress Currently a work in progress non-breaking Non-breaking change labels Jan 22, 2024
@rjzamora rjzamora self-assigned this Jan 22, 2024
@github-actions github-actions bot added the Python Affects Python cuDF API. label Jan 22, 2024
@rjzamora rjzamora marked this pull request as ready for review January 30, 2024 20:18
@rjzamora rjzamora requested review from a team as code owners January 30, 2024 20:18
@rjzamora rjzamora added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Jan 30, 2024
@rjzamora
Copy link
Member Author

rjzamora commented Jan 30, 2024 •
edited
Loading

I moved the tokenize tests to cudf, since they don't actually rely on any dask-cudf (and to make codecov a bit happier). However, I'll be happy to revert that change if anyone objects :) (EDIT: Never mind. We don't always have dask-cudf installed when we run the cudf tests, so the coverage was better before)

@rjzamora rjzamora added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Jan 31, 2024
@rjzamora
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit bb59715 into rapidsai:branch-24.04 Jan 31, 2024
69 checks passed
@rjzamora rjzamora deleted the fix-dask-normalize branch January 31, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charlesbluca charlesbluca charlesbluca approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

@vyasr vyasr Awaiting requested review from vyasr vyasr 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

@rjzamora rjzamora

Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rjzamora @galipremsagar @charlesbluca