Skip to content

[SPARK-26127][ML] Remove deprecated setters from tree regression and classification models #23093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mgaido91 wants to merge 4 commits into from
Closed

[SPARK-26127][ML] Remove deprecated setters from tree regression and classification models #23093

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56d61f3
[SPARK-26127][ML] Remove deprecated setImpurity from tree regression …
mgaido91 Nov 20, 2018
8e78883
remove all deprecated setters in treeParams
mgaido91 Nov 20, 2018
ce4a02e
fix mima
mgaido91 Nov 20, 2018
762e4de
Merge branch 'master' into SPARK-26127
mgaido91 Nov 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/classification/DecisionTreeClassifier.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class DecisionTreeClassifier @Since("1.4.0") (

/** @group setParam */
@Since("1.4.0")
override def setImpurity(value: String): this.type = set(impurity, value)
def setImpurity(value: String): this.type = set(impurity, value)
Copy link
Member

@srowen srowen Nov 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove these too. I assumed they were meant to be deprecated because the trait's implementation was deprecated, and there's no compelling reason to only remove it from the trait. The point was to use set().

Copy link
Contributor Author

@mgaido91 mgaido91 Nov 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think those can be removed. The ones which were deprecated are the setImpurity on the Models, not on the Classifiers/Regressors which build the models.

Copy link
Member

@srowen srowen Nov 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I think I'm wrong about the logic, given the first attempt to do this in #17867 Yes this is fine. In fact I think you're welcome to just revive that PR and remove a lot of similar deprecated methods again.

Copy link
Contributor Author

@mgaido91 mgaido91 Nov 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks. I'll update this PR removing all the other deprecated items for these models then. Thanks.


/** @group setParam */
@Since("1.6.0")
Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/classification/GBTClassifier.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class GBTClassifier @Since("1.4.0") (
* @group setParam
*/
@Since("1.4.0")
override def setImpurity(value: String): this.type = {
def setImpurity(value: String): this.type = {
logWarning("GBTClassifier.setImpurity should NOT be used")
this
}
Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/classification/RandomForestClassifier.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class RandomForestClassifier @Since("1.4.0") (

/** @group setParam */
@Since("1.4.0")
override def setImpurity(value: String): this.type = set(impurity, value)
def setImpurity(value: String): this.type = set(impurity, value)

// Parameters from TreeEnsembleParams:

Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/regression/DecisionTreeRegressor.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class DecisionTreeRegressor @Since("1.4.0") (@Since("1.4.0") override val uid: S

/** @group setParam */
@Since("1.4.0")
override def setImpurity(value: String): this.type = set(impurity, value)
def setImpurity(value: String): this.type = set(impurity, value)

/** @group setParam */
@Since("1.6.0")
Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/regression/GBTRegressor.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class GBTRegressor @Since("1.4.0") (@Since("1.4.0") override val uid: String)
* @group setParam
*/
@Since("1.4.0")
override def setImpurity(value: String): this.type = {
def setImpurity(value: String): this.type = {
logWarning("GBTRegressor.setImpurity should NOT be used")
this
}
Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/regression/RandomForestRegressor.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class RandomForestRegressor @Since("1.4.0") (@Since("1.4.0") override val uid: S

/** @group setParam */
@Since("1.4.0")
override def setImpurity(value: String): this.type = set(impurity, value)
def setImpurity(value: String): this.type = set(impurity, value)

// Parameters from TreeEnsembleParams:

Expand Down
14 changes: 0 additions & 14 deletions mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,6 @@ private[ml] trait TreeClassifierParams extends Params {

setDefault(impurity -> "gini")

/**
* @deprecated This method is deprecated and will be removed in 3.0.0.
* @group setParam
*/
@deprecated("This method is deprecated and will be removed in 3.0.0.", "2.1.0")
def setImpurity(value: String): this.type = set(impurity, value)

/** @group getParam */
final def getImpurity: String = $(impurity).toLowerCase(Locale.ROOT)

Expand Down Expand Up @@ -273,13 +266,6 @@ private[ml] trait HasVarianceImpurity extends Params {

setDefault(impurity -> "variance")

/**
* @deprecated This method is deprecated and will be removed in 3.0.0.
* @group setParam
*/
@deprecated("This method is deprecated and will be removed in 3.0.0.", "2.1.0")
def setImpurity(value: String): this.type = set(impurity, value)

/** @group getParam */
final def getImpurity: String = $(impurity).toLowerCase(Locale.ROOT)

Expand Down
13 changes: 9 additions & 4 deletions project/MimaExcludes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,26 @@ object MimaExcludes {

// Exclude rules for 3.0.x
lazy val v30excludes = v24excludes ++ Seq(

// [SPARK-26127] Remove deprecated setImpurity from tree regression and classification models
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.ml.classification.DecisionTreeClassificationModel.setImpurity"),
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.ml.classification.RandomForestClassificationModel.setImpurity"),
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.ml.classification.GBTClassificationModel.setImpurity"),
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.ml.regression.DecisionTreeRegressionModel.setImpurity"),
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.ml.regression.GBTRegressionModel.setImpurity"),
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.ml.regression.RandomForestRegressionModel.setImpurity"),

// [SPARK-26090] Resolve most miscellaneous deprecation and build warnings for Spark 3
ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.mllib.stat.test.BinarySampleBeanInfo"),
ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.mllib.regression.LabeledPointBeanInfo"),
ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.ml.feature.LabeledPointBeanInfo"),

// [SPARK-25959] GBTClassifier picks wrong impurity stats on loading
ProblemFilters.exclude[IncompatibleResultTypeProblem]("org.apache.spark.ml.classification.GBTClassificationModel.setImpurity"),
ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.tree.HasVarianceImpurity.org$apache$spark$ml$tree$HasVarianceImpurity$_setter_$impurity_="),
ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.tree.HasVarianceImpurity.org$apache$spark$ml$tree$HasVarianceImpurity$_setter_$impurity_="),
ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.tree.HasVarianceImpurity.org$apache$spark$ml$tree$HasVarianceImpurity$_setter_$impurity_="),
ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.tree.HasVarianceImpurity.org$apache$spark$ml$tree$HasVarianceImpurity$_setter_$impurity_="),
ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.tree.HasVarianceImpurity.org$apache$spark$ml$tree$HasVarianceImpurity$_setter_$impurity_="),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("org.apache.spark.ml.regression.DecisionTreeRegressionModel.setImpurity"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("org.apache.spark.ml.regression.GBTRegressionModel.setImpurity"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("org.apache.spark.ml.regression.RandomForestRegressionModel.setImpurity"),

// [SPARK-25908][CORE][SQL] Remove old deprecated items in Spark 3
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.BarrierTaskContext.isRunningLocally"),
Expand Down