Github user mpjlu commented on the issue:
https://github.com/apache/spark/pull/15214
Hi @srowen and @yanboliang ; Thanks for your following up PR.
I partly agree with your comments on 17017.
**1. "if users both set numTopFeatures and percentile, it will train kbest
or percentile model based on the order of setting (the latter setting one will
be trained). This make users confused, and we should allow users to set
selector type explicitly."**
For the user confused you mentioned here, I think the main reason is
function name. I have changed the function name of setAlpha to setFPR in
SPARK-17645. setNumTopFeature should be setKBest.
By this change, it can be much clear.
For example, setKBest(100), setPercentile(0.1), setFPR(0.05). The selection
type and parameters is very clear by one function.
But for your method, user have to strike
"setSelectorType("KBest").setNumTopFeatures(100)" to do the same thing as
"setKBest(100)"
**2. "if there are more than one parameter except alpha can be set for fpr
model, we can not handle it elegantly in the existing framework. And similar
issues for kbest and percentile model. "**
I cannot think out any other parameters for fpr, kbest, percentile now. But
if there is, I think it is just the same thing as your method. for example,
setKBest(100).setOther(),,
I agree with you for other change. Thanks very much.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
