-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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-28780][ML][2.4] deprecate LinearSVCModel.setWeightCol #25547
[SPARK-28780][ML][2.4] deprecate LinearSVCModel.setWeightCol #25547
Conversation
Thank you, @zhengruifeng ! |
@@ -313,7 +313,8 @@ class LinearSVCModel private[classification] ( | |||
setDefault(threshold, 0.0) | |||
|
|||
@Since("2.2.0") | |||
def setWeightCol(value: Double): this.type = set(threshold, value) | |||
@deprecated("This method is deprecated and will be removed in the future.", "2.4.4") |
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.
Since we already removed this, shall we use in 3.0.0
instead of in the future
?
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 am not sure but 2.4.5
maybe anothor option?
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.
Nope. That's radical. We don't remove API in maintenance
release. 3.0.0
is natural for this kind of removal. I mean the semantic version, major.minor.maintenance
.
All releases with the same major version number will have API compatibility
cc @srowen |
Test build #109549 has finished for PR 25547 at commit
|
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.
Please address the comment.
updated. Thanks for reviewing and explanation! @dongjoon-hyun |
Test build #109558 has finished for PR 25547 at commit
|
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.
+1, LGTM. Merged to branch-2.4
.
This PR already pass the Jenkins and the last commit is only string change at deprecation message.
Thank you for update, @zhengruifeng .
### What changes were proposed in this pull request? deprecate `LinearSVCModel.setWeightCol`, and make it a no-op ### Why are the changes needed? `LinearSVCModel` should not provide this setter, moreover, this method is wrongly defined. ### Does this PR introduce any user-facing change? no, this method is only deprecated ### How was this patch tested? existing suites Closes #25547 from zhengruifeng/svc_model_deprecate_setWeight_2.4. Authored-by: zhengruifeng <ruifengz@foxmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
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.
Also looks good.
### What changes were proposed in this pull request? deprecate `LinearSVCModel.setWeightCol`, and make it a no-op ### Why are the changes needed? `LinearSVCModel` should not provide this setter, moreover, this method is wrongly defined. ### Does this PR introduce any user-facing change? no, this method is only deprecated ### How was this patch tested? existing suites Closes apache#25547 from zhengruifeng/svc_model_deprecate_setWeight_2.4. Authored-by: zhengruifeng <ruifengz@foxmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
### What changes were proposed in this pull request? deprecate `LinearSVCModel.setWeightCol`, and make it a no-op ### Why are the changes needed? `LinearSVCModel` should not provide this setter, moreover, this method is wrongly defined. ### Does this PR introduce any user-facing change? no, this method is only deprecated ### How was this patch tested? existing suites Closes apache#25547 from zhengruifeng/svc_model_deprecate_setWeight_2.4. Authored-by: zhengruifeng <ruifengz@foxmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
What changes were proposed in this pull request?
deprecate
LinearSVCModel.setWeightCol
, and make it a no-opWhy are the changes needed?
LinearSVCModel
should not provide this setter, moreover, this method is wrongly defined.Does this PR introduce any user-facing change?
no, this method is only deprecated
How was this patch tested?
existing suites