-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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.
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
[SPARK-26127][ML] Remove deprecated setters from tree regression and classification models #23093
Conversation
…and classification models
cc @srowen |
Test build #99047 has finished for PR 23093 at commit
|
@@ -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) |
There was a problem hiding this comment.
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().
There was a problem hiding this comment.
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 Model
s, not on the Classifier
s/Regressor
s which build the models.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Test build #99059 has finished for PR 23093 at commit
|
Test build #99060 has finished for PR 23093 at commit
|
Test build #99108 has finished for PR 23093 at commit
|
Merged to master |
…classification models ## What changes were proposed in this pull request? The setter methods are deprecated since 2.1 for the models of regression and classification using trees. The deprecation was stating that the method would have been removed in 3.0. Hence the PR removes the deprecated method. ## How was this patch tested? NA Closes apache#23093 from mgaido91/SPARK-26127. Authored-by: Marco Gaido <marcogaido91@gmail.com> Signed-off-by: Sean Owen <sean.owen@databricks.com>
What changes were proposed in this pull request?
The setter methods are deprecated since 2.1 for the models of regression and classification using trees. The deprecation was stating that the method would have been removed in 3.0. Hence the PR removes the deprecated method.
How was this patch tested?
NA