MNT Deprecate unused parameter from OneClassSVM fit method.#20843
MNT Deprecate unused parameter from OneClassSVM fit method.#20843glemaitre merged 8 commits intoscikit-learn:mainfrom
Conversation
thomasjpfan
left a comment
There was a problem hiding this comment.
Thank you for the PR @jmloyola !
LGTM
ogrisel
left a comment
There was a problem hiding this comment.
I theory we should go through a deprecation cycle for this change. That means raising a warning to warn that the fit method will no longer accept these useless parameters in version 1.2.
|
@ogrisel, @thomasjpfan, upon further inspection, I'm not that certain that the removal of from sklearn import svm
X = [[-2, -1], [-1, -1], [-1, -2], [1, 1], [1, 2], [2, 1]]
clf = svm.OneClassSVM()
clf.fit(X, extra_param='unused')The following error is raised:
To my knowledge, there is no extra keyword parameter that BaseLibSVM's fit method could accept. A couple of years ago the parameter Please, let me know what do you think. Should we continue with the deprecation cycle or not? |
Would we not consider this as a bug fix instead. For instance, a small typo in |
|
After discussing with @ogrisel IRL, I am fine with the deprecation cycle. If a user makes a mistake in the spelling, he/she will at least get the warning that something is wrong. |
sklearn/svm/_classes.py
Outdated
| ) | ||
|
|
||
| def fit(self, X, y=None, sample_weight=None, **params): | ||
| def fit(self, X, y=None, sample_weight=None): |
There was a problem hiding this comment.
| def fit(self, X, y=None, sample_weight=None): | |
| def fit(self, X, y=None, sample_weight=None, **params): |
Co-authored-by: glemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
|
Thanks for the suggestions @glemaitre 🤓 |
Co-authored-by: glemaitre <g.lemaitre58@gmail.com>
…_new add backticks (scikit-learn#20914), deprecate **params in fit (scikit-learn#20843), add feature_names_in_ (scikit-learn#20787)
…_new add backticks (scikit-learn#20914), deprecate **params in fit (scikit-learn#20843), add feature_names_in_ (scikit-learn#20787) uncompromisingly reformat plot_oneclass_vs_svdd with black
…earn#20843) Co-authored-by: glemaitre <g.lemaitre58@gmail.com>
…_new add backticks (scikit-learn#20914), deprecate **params in fit (scikit-learn#20843), add feature_names_in_ (scikit-learn#20787) uncompromisingly reformat plot_oneclass_vs_svdd with black
…_new add backticks (scikit-learn#20914), deprecate **params in fit (scikit-learn#20843), add feature_names_in_ (scikit-learn#20787) uncompromisingly reformat plot_oneclass_vs_svdd with black
…_new add backticks (scikit-learn#20914), deprecate **params in fit (scikit-learn#20843), add feature_names_in_ (scikit-learn#20787) uncompromisingly reformat plot_oneclass_vs_svdd with black
…cSVM scikit-learn#24001) finish v1.2 deprecation of params kwargs in `.fit` of SVDD (similar to ocSVM scikit-learn#20843) removed SVDD param-validation exception from test_common.py since scikit-learn#23462 is go (scikit-learn#22722)
…_new add backticks (scikit-learn#20914), deprecate **params in fit (scikit-learn#20843), add feature_names_in_ (scikit-learn#20787) uncompromisingly reformat plot_oneclass_vs_svdd with black
…cSVM scikit-learn#24001) finish v1.2 deprecation of params kwargs in `.fit` of SVDD (similar to ocSVM scikit-learn#20843) TST ensure SVDD passes param-validation test_common.py due to scikit-learn#23462 (scikit-learn#22722)
Reference Issues/PRs
Addresses a comment given in this PR.
What does this implement/fix? Explain your changes.
This PR
removesdeprecates the parameterparamsfrom OneClassSVM's fit method since the BaseLibSVM's fit method does not have any more parameters than the already passed as keywords (X, y, sample_weight).