srowen commented on a change in pull request #25383: [SPARK-13677][ML]
Implement Tree-Based Feature Transformation for ML
URL: https://github.com/apache/spark/pull/25383#discussion_r313900977
##########
File path:
mllib/src/main/scala/org/apache/spark/ml/classification/DecisionTreeClassifier.scala
##########
@@ -55,6 +55,10 @@ class DecisionTreeClassifier @Since("1.4.0") (
// Override parameter setters from parent trait for Java API compatibility.
+ /** @group setParam */
+ @Since("3.0.0")
+ def setLeafCol(value: String): this.type = set(leafCol, value)
Review comment:
Yes, agree, it shouldn't be in the model classes. So
`DecisionTreeClassifierParams` doesn't help. Hm... per the discussion below, I
agree it's extra weight to refactor the common elements into a superclass of
two decision tree classifiers, but it might well be worth it. It looks like it
would save a few hundred lines of duplicated code? that would mitigate the
concern about the large change here. I'm lightly in favor of going that way. I
wouldn't do it for 10 lines of code or something.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
