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

[bugfix] Fix duplicated nodes in maximal_independent_set output #5692

Merged
merged 2 commits into from Apr 3, 2019

Conversation

@tpoterba
Copy link
Collaborator

@tpoterba tpoterba commented Mar 26, 2019

Fixes #5535

Copy link
Collaborator

@danking danking left a comment

this needs to distinct before the if too. Neither case should produce duplicates

@tpoterba
Copy link
Collaborator Author

@tpoterba tpoterba commented Mar 26, 2019

won't this require keying (and therefore shuffling) then?

@catoverdrive
Copy link
Contributor

@catoverdrive catoverdrive commented Apr 1, 2019

@danking this PR is showing up as approved on the CI, but not as approved here.

@tpoterba
Copy link
Collaborator Author

@tpoterba tpoterba commented Apr 1, 2019

github API troubles?

Copy link
Collaborator

@cseed cseed left a comment

Requesting changes to get the CI unstuck.

@cseed
Copy link
Collaborator

@cseed cseed commented Apr 3, 2019

This fixes the CI: #5757. I will dismiss my review after that goes in.

@danking
Copy link
Collaborator

@danking danking commented Apr 3, 2019

won't this require keying (and therefore shuffling) then?

@tpoterba yeah but that's the correct answer no?

@danking
Copy link
Collaborator

@danking danking commented Apr 3, 2019

@tpoterba
Copy link
Collaborator Author

@tpoterba tpoterba commented Apr 3, 2019

yeah, the keyed=False path is for performance and I'm OK leaving that with duplicated nodes

@danking
Copy link
Collaborator

@danking danking commented Apr 3, 2019

@tpoterba if you're good with the verbiage I added to the notes, then I'll approve.

@tpoterba
Copy link
Collaborator Author

@tpoterba tpoterba commented Apr 3, 2019

ya

danking
danking approved these changes Apr 3, 2019
@danking danking merged commit 61a0751 into hail-is:master Apr 3, 2019
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants