-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
ENH: cluster: add an optional argument seed
for kmeans
and kmeans2
to set random generator and random state.
#13972
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
Conversation
… to set random generator and random state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good addition, thanks! A few minor things on my side.
Also, I am not sure what we should use in the future to name RNG. seed
or random_state
or something else? @mdhaber @WarrenWeckesser (we could add this to your PR about best practice)?
Co-authored-by: Pamphile ROY <roy.pamphile@gmail.com>
Co-authored-by: Pamphile ROY <roy.pamphile@gmail.com>
Co-authored-by: Pamphile ROY <roy.pamphile@gmail.com>
Co-authored-by: Pamphile ROY <roy.pamphile@gmail.com>
Co-authored-by: Pamphile ROY <roy.pamphile@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with the change on the doc to respect the line length. Thanks! I will wait for the CI and merge unless you want to add something.
The timeout are unrelated and seen in other PRs so merging. Thanks again @AtsushiSakai |
Reference issue
fix #9450
What does this implement/fix?
I added an optional argument
seed
forkmeans
andkmeans2
to set random generator and random state.I also added tests for these and updated docs.