-
-
Notifications
You must be signed in to change notification settings - Fork 26.2k
FIX don't modify sample weight inplace in KMeans #17204
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
FIX don't modify sample weight inplace in KMeans #17204
Conversation
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.
Thanks! Please add a what's new entry.
...................... | ||
|
||
- |Fix| Fixed a bug in :class:`cluster.KMeans` where the sample weights | ||
provided by the user was modified in place. :pr:`17204` by |
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.
provided by the user was modified in place. :pr:`17204` by | |
provided by the user were modified in place. :pr:`17204` by |
@@ -177,7 +177,7 @@ def _check_normalize_sample_weight(sample_weight, X): | |||
# an array of 1 (i.e. samples_weight is None) is already normalized | |||
n_samples = len(sample_weight) | |||
scale = n_samples / sample_weight.sum() | |||
sample_weight *= scale | |||
sample_weight = sample_weight * scale |
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.
maybe make an explicit copy above instead?
I'll justmerge and will fix the typo in #17205 |
Fixes #17203
KMeans scales sample weight to sum up to n_samples. It has been done inplace when we generalized the use of check_sample_weight.
@rth