Skip to content

Conversation

AtsushiSakai
Copy link
Member

Reference issue

fix #9450

What does this implement/fix?

I added an optional argument seed for kmeans and kmeans2 to set random generator and random state.
I also added tests for these and updated docs.

… to set random generator and random state.
@AtsushiSakai AtsushiSakai added scipy.cluster enhancement A new feature or improvement labels May 2, 2021
Copy link
Member

@tupui tupui left a 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)?

AtsushiSakai and others added 7 commits May 3, 2021 09:11
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>
Copy link
Member

@tupui tupui left a 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.

@tupui
Copy link
Member

tupui commented May 4, 2021

The timeout are unrelated and seen in other PRs so merging. Thanks again @AtsushiSakai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.cluster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allow seeding of init methods in vq.kmeans2
2 participants