Skip to content

ENH Deprecate class_weight_ in regression/single class models in SVM module #22898

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

Merged
merged 32 commits into from
May 24, 2022

Conversation

Micky774
Copy link
Contributor

@Micky774 Micky774 commented Mar 19, 2022

Reference Issues/PRs

Fixes #14788

What does this implement/fix? Explain your changes.

Began deprecation of some properties in svm module

  • Removed instation of class_weight_ = np.empty(0) in BaseLibSVM
  • Added new property _class_weight in BaseLibSVM to serve as an intermediate for deprecation by avoiding direct calls to class_weight_ for SVR, NuSVR, and OneClassSVM
  • Added test for deprecated attribute

Any other comments?

- Removed instation of `class_weight_ = np.empty(0)` in `BaseLibSVM`
- Added deprecated properties for `class_weight_` and `n_support_`
  in `SVR`, `NuSVR`, and `OneClassSVM`.
@Micky774 Micky774 marked this pull request as draft March 20, 2022 01:38
@Micky774 Micky774 marked this pull request as ready for review March 20, 2022 22:11
Micky774 and others added 4 commits March 25, 2022 16:40
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
- Updated tests to use `n_support_[0]` directly
- Added deprecated property to `SVR, NuSVR, OneClassSVM`
- Removed partial `n_support_` deprecation in parent `BaseLibSVM`
- Removed reliance on `n_support_` in `_validate_for_predict`
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the update!

@Micky774 Micky774 changed the title Deprecate class_weight_ and n_support_ in regression/single class models in SVM module Deprecate class_weight_ in regression/single class models in SVM module May 12, 2022
@Micky774 Micky774 changed the title Deprecate class_weight_ in regression/single class models in SVM module ENH Deprecate class_weight_ in regression/single class models in SVM module May 12, 2022
Copy link
Member

@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

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

Thank you for your this contribution, @Micky774.

Here are a few comments.

Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Minor comment, otherwise LGTM

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

It took me time to get it but it looks correct and the tests pass without raising the warning when it should not be raised.

The trick to look at the _validate_targets method that is overridden in the classifier base class BaseSVC that defines the public class_weight_ only for classifiers.

@ogrisel ogrisel merged commit f205b95 into scikit-learn:main May 24, 2022
@Micky774 Micky774 deleted the deprecate_in_svr branch May 24, 2022 19:20
ivannz added a commit to ivannz/scikit-learn that referenced this pull request Jun 14, 2022
ivannz added a commit to ivannz/scikit-learn that referenced this pull request Aug 29, 2022
ivannz added a commit to ivannz/scikit-learn that referenced this pull request Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate n_support_ and class_weight_ from SVR class
4 participants