Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14597#discussion_r76049523
--- Diff:
mllib/src/main/scala/org/apache/spark/mllib/feature/ChiSqSelector.scala ---
@@ -189,11 +232,21 @@ class ChiSqSelector @Since("1.3.0") (
*/
@Since("1.3.0")
def fit(data: RDD[LabeledPoint]): ChiSqSelectorModel = {
- val indices = Statistics.chiSqTest(data)
- .zipWithIndex.sortBy { case (res, _) => -res.statistic }
- .take(numTopFeatures)
- .map { case (_, indices) => indices }
- .sorted
+ val chiSqTestResult = Statistics.chiSqTest(data)
+ val features = selectorType match {
+ case ChiSqSelectorType.KBest => chiSqTestResult
+ .zipWithIndex.sortBy { case (res, _) => -res.statistic }
--- End diff --
I think it would still be better to sort here since it's actually simpler
and not expensive. Do you feel strongly about it?
Otherwise, I have just one last thing I noted when reading all the code
again -- how about changing "Fpr" to "FPR" because it does stand for "False
Positive Rate"?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
