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

[REVIEW] fea/hash_index #1917

Closed
wants to merge 4 commits into from
Closed

Conversation

quasiben
Copy link
Member

@quasiben quasiben commented Jun 4, 2019

This PR adds method to create a hash of the index. This is a first step for resolving failures in rapidsai/dask-cudf#262

cc @kkraus14

@quasiben quasiben requested a review from a team as a code owner June 4, 2019 01:54
# hash_columns produces negative valuesg
# probably can switch to np.uint32
# when supported by libcud
return abs(sr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to confirm, you want to return a Series here and not a subtype of Index?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so -- i need to build some things in dask to properly check now

@kkraus14 kkraus14 added 3 - Ready for Review Ready for review by team 4 - Needs cuDF Reviewer Python Affects Python cuDF API. labels Jun 4, 2019
Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

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

You need to update your RMM submodule, otherwise this looks good to go

@kkraus14 kkraus14 changed the title fea/hash_index [REVIEW] fea/hash_index Jun 4, 2019
@kkraus14
Copy link
Collaborator

kkraus14 commented Jun 4, 2019

@quasiben I'm going to hold off on merging this until you give the go ahead that no more changes are needed to support dask.

@kkraus14 kkraus14 added 0 - Waiting on Author Waiting for author to respond to review and removed 4 - Needs cuDF Reviewer 3 - Ready for Review Ready for review by team labels Jun 4, 2019
@quasiben
Copy link
Member Author

quasiben commented Jun 4, 2019

@kkraus14, thanks. I think we should hold off as well

@quasiben
Copy link
Member Author

quasiben commented Jun 4, 2019

I think #1720 may be blocking this PR.

Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

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

Changing review just so it's unable to be merged.

@kkraus14
Copy link
Collaborator

Going to close this for now and we can reopen in the future if we want/need to.

@kkraus14 kkraus14 closed this Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - Waiting on Author Waiting for author to respond to review Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants