Github user VinceShieh commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14858#discussion_r78513514
  
    --- Diff: 
mllib/src/main/scala/org/apache/spark/ml/feature/QuantileDiscretizer.scala ---
    @@ -109,7 +114,7 @@ final class QuantileDiscretizer @Since("1.6.0") 
(@Since("1.6.0") override val ui
       @Since("2.0.0")
       override def fit(dataset: Dataset[_]): Bucketizer = {
         transformSchema(dataset.schema, logging = true)
    -    val splits = dataset.stat.approxQuantile($(inputCol),
    +    val splits = 
dataset.select($(inputCol)).na.drop().stat.approxQuantile($(inputCol),
    --- End diff --
    
    @srowen I have one question here. The reason why we dont move this NaN 
filter into approxiQuantile or multipleApproxQuantiles is that, those apis are 
shared with sparkSQL? Becoz, personally I think it would look better if we put 
this filter inside multipleApproxQuantiles, though it would introduce more 
changes and,  should make sure it doesnt impact other components other than 
mllib.


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

Reply via email to