-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-11559] [MLlib] Make runs no effect in mllib.KMeans
#12608
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
|
Test build #56695 has finished for PR 12608 at commit
|
| * This function has no effect since Spark 2.0.0. | ||
| */ | ||
| @Since("1.4.0") | ||
| @deprecated("Support for runs is deprecated. This param will have no effect in 2.0.0.", "1.6.0") |
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.
isn't it better to keep the @deprecated?
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.
@deprecated means it take effect but not recommend to use, but in this PR we will make it no effect.
|
I'll take a look |
| (default: None) | ||
| """ | ||
| if runs != 1: | ||
| warnings.warn( |
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.
I'd keep a warning, though you can remove the word "deprecated"
|
|
That's all. Thanks for separating this out. |
|
Test build #56997 has finished for PR 12608 at commit
|
|
LGTM |
What changes were proposed in this pull request?
We deprecated
runsof mllib.KMeans in Spark 1.6 (SPARK-11358). In 2.0, we will make it no effect (with warning messages). We did not removesetRuns/getRunsfor better binary compatibility.This PR change
runswhich are appeared at the public API. Usage inside ofKMeans.runAlgorithm()will be resolved at #10806.How was this patch tested?
Existing unit tests.
cc @jkbradley