-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-18318][ML] ML, Graph 2.1 QA: API: New Scala APIs, docs #16009
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
Changes from 7 commits
8cfc859
ac72c05
8c36940
f494a47
0b1cced
eae0d2c
019e5af
27b07ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,15 +49,13 @@ private[feature] trait ChiSqSelectorParams extends Params | |
| * | ||
| * @group param | ||
| */ | ||
| @Since("1.6.0") | ||
| final val numTopFeatures = new IntParam(this, "numTopFeatures", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually we don't add
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This happens in several other places though, are we going to remove them all?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, theoretically we should do that, but I'm not very confidence whether this change is appropriate. If we meet an agreement on how to deal with this issue, we can address other places in this PR or follow-up work. cc @jkbradley
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case, I think it's OK to have Since annotations in the trait since it is private and should never be used beyond ChiSqSelector and ChiSqSelectorModel. That seems pretty safe to me.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's safe for this case. However, I found lots of other traits which is also safe enough to add since tag but did not add. I reverted this part of change in this PR to make it catch another RC of 2.1, and I think we should unify them in a separate work. Thanks. |
||
| "Number of features that selector will select, ordered by ascending p-value. If the" + | ||
| " number of features is < numTopFeatures, then this will select all features.", | ||
| ParamValidators.gtEq(1)) | ||
| setDefault(numTopFeatures -> 50) | ||
|
|
||
| /** @group getParam */ | ||
| @Since("1.6.0") | ||
| def getNumTopFeatures: Int = $(numTopFeatures) | ||
|
|
||
| /** | ||
|
|
@@ -66,14 +64,12 @@ private[feature] trait ChiSqSelectorParams extends Params | |
| * Default value is 0.1. | ||
| * @group param | ||
| */ | ||
| @Since("2.1.0") | ||
| final val percentile = new DoubleParam(this, "percentile", | ||
| "Percentile of features that selector will select, ordered by ascending p-value.", | ||
| ParamValidators.inRange(0, 1)) | ||
| setDefault(percentile -> 0.1) | ||
|
|
||
| /** @group getParam */ | ||
| @Since("2.1.0") | ||
| def getPercentile: Double = $(percentile) | ||
|
|
||
| /** | ||
|
|
@@ -94,15 +90,13 @@ private[feature] trait ChiSqSelectorParams extends Params | |
| * Supported options: "numTopFeatures" (default), "percentile", "fpr". | ||
| * @group param | ||
| */ | ||
| @Since("2.1.0") | ||
| final val selectorType = new Param[String](this, "selectorType", | ||
| "The selector type of the ChisqSelector. " + | ||
| "Supported options: " + OldChiSqSelector.supportedSelectorTypes.mkString(", "), | ||
| ParamValidators.inArray[String](OldChiSqSelector.supportedSelectorTypes)) | ||
| setDefault(selectorType -> OldChiSqSelector.NumTopFeatures) | ||
|
|
||
| /** @group getParam */ | ||
| @Since("2.1.0") | ||
| def getSelectorType: String = $(selectorType) | ||
| } | ||
|
|
||
|
|
||
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.
"from the column when" -> "from the column during"
"model for making prediction and transformation" -> "model for making predictions"