-
Notifications
You must be signed in to change notification settings - Fork 28.6k
[SPARK-6268][MLlib] KMeans parameter getter methods #4974
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 #28460 has started for PR 4974 at commit
|
Test build #28460 has finished for PR 4974 at commit
|
Test PASSed. |
@@ -78,6 +93,11 @@ class KMeans private ( | |||
} | |||
|
|||
/** | |||
* Number of runs of the algorithm to execute in parallel. | |||
*/ | |||
def getRuns: Int = runs |
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.
setRuns
is an experimental API, so it should be the same for getRuns
.
LGTM except one minor inline comment. |
Test build #28466 has started for PR 4974 at commit
|
@mengxr Thanks for review. New commit submitted. |
Test build #28466 has finished for PR 4974 at commit
|
Test PASSed. |
@@ -52,12 +52,22 @@ class KMeans private ( | |||
*/ | |||
def this() = this(2, 20, 1, KMeans.K_MEANS_PARALLEL, 5, 1e-4, Utils.random.nextLong()) | |||
|
|||
/** | |||
* Number of clusters to create (k). |
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.
Nit: You could prefix this with @return
Interesting question: naming getters getFoo
is of course standard in Java, and seems just fine too, although in Scala we'd be more used to accessing foo
only. It's possible to declare private var _foo
and then declare the public getter to have name foo
here. Does anyone prefer that?
Hi @srowen Thanks for the suggestions.
|
Ignore my second comment; it would take a fair bit more change including changing current setters. |
Merged into master. Thanks! |
jira: https://issues.apache.org/jira/browse/SPARK-6268
KMeans has many setters for parameters. It should have matching getters.