Github user viirya commented on a diff in the pull request: https://github.com/apache/spark/pull/19715#discussion_r150401598 --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/QuantileDiscretizer.scala --- @@ -104,7 +126,8 @@ private[feature] trait QuantileDiscretizerBase extends Params */ @Since("1.6.0") final class QuantileDiscretizer @Since("1.6.0") (@Since("1.6.0") override val uid: String) - extends Estimator[Bucketizer] with QuantileDiscretizerBase with DefaultParamsWritable { + extends Estimator[Bucketizer] with QuantileDiscretizerBase with DefaultParamsWritable + with HasInputCols with HasOutputCols { --- End diff -- It looks a bit weird to have `HasInputCols` and `HasOutputCols` directly in `QuantileDiscretizer` and leave other params in `QuantileDiscretizerBase`. But extending `HasInputCols` and `HasOutputCols` in `QuantileDiscretizerBase` causes binary compatibility issue. I think we don't want to break the compatibility in the effort of adding multi-col support.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org