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