Github user mpjlu commented on the issue:

    https://github.com/apache/spark/pull/15214
  
    hi @srowen .
    My understand of yanbo's comments here is,
    if user use chSqSelector like this:
    model1 = new ChiSqSelector().setFPR(0.05).setKBest(100).fit(data)
    model2 = new ChiSqSelector().setKBest(100).setFPR(0.05).fit(data)
    model1 will be different with model2. so the model is dependent on the 
order of users setting params.
    Actually, user should not use ChiSqSelector like this. One just need to set 
one SelectorType/Parameter is ok.  But if one don't know ChiSqSelector, he may 
do like this. So yanbo think this is a problem. 
    
    In this PR, setFPR(0.05) is split to two functions: 
setSelectorType("fpr").setAlpha(0.05). This maybe clear to the user.  
    By the principle of software development: one function do one thing, I am 
ok with this change.  
    But from user experience,  I like the spark-17017 method.  



---
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

Reply via email to