Skip to content
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

Jump to bottom

DOC Ensures that OneClassSVM passes numpydoc validation #20463

Merged
merged 5 commits into from
Jul 13, 2021
Merged

DOC Ensures that OneClassSVM passes numpydoc validation #20463

merged 5 commits into from
Jul 13, 2021

Conversation

jmloyola
Copy link
Member

@jmloyola jmloyola commented Jul 3, 2021

Reference Issues/PRs

Addresses #20308

What does this implement/fix? Explain your changes.

This PR ensures OneClassSVM is compatible with numpydoc.

  • Remove OneClassSVM from DOCSTRING_IGNORE_LIST.
  • Verify all tests passing.

Any other comments?

For this PR to pass all numpydoc test, this PR should be merged before.

#DataUmbrella Sprint

jmloyola
jmloyola commented Jul 4, 2021
View reviewed changes
sklearn/svm/_classes.py
Comment on lines +1564 to +1565
**params : dict
Additional fit parameters.
Copy link
Member Author

Choose a reason for hiding this comment

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

To my knowledge, the fit function in the hierarchy of this class doesn't have any more parameters. Thus, params is not actually needed. Should we remove it?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah looks like it comes from BaseLibSVM. If you want you can check that passing any parameter actually raises an error, but it should be save to remove.

Copy link
Member

Choose a reason for hiding this comment

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

I would do this change in another PR thought.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. When we are done with this PR, I will try to fix that in another PR.

@thomasjpfan thomasjpfan mentioned this pull request Jul 6, 2021
Closed
amueller
amueller previously approved these changes Jul 10, 2021
View reviewed changes
@amueller amueller dismissed their stale review July 10, 2021 15:59

actually still failing

@jmloyola
Copy link
Member Author

Yeah. Once this PR get merged, these issues will be resolved.

@glemaitre
Copy link
Member

You need to document the property coef_ and n_support_

glemaitre
glemaitre requested changes Jul 13, 2021
View reviewed changes
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

A couple of additional changes

sklearn/svm/_classes.py Outdated Show resolved Hide resolved
sklearn/svm/_classes.py
Comment on lines +1564 to +1565
**params : dict
Additional fit parameters.
Copy link
Member

Choose a reason for hiding this comment

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

I would do this change in another PR thought.

@glemaitre glemaitre changed the title [WIP] DOC Ensures that OneClassSVM passes numpydoc validation DOC Ensures that OneClassSVM passes numpydoc validation Jul 13, 2021
@jmloyola @glemaitre
Update sklearn/svm/_classes.py
24b5651 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
@jmloyola
Copy link
Member Author

Thanks for the comments @glemaitre.
Once this PR gets merged, the properties will be documented.
Let me know if there is something else I need to do.

@glemaitre
Copy link
Member

#20457 was merged. You can merge main into your branch.

@glemaitre glemaitre merged commit 85b0b54 into scikit-learn:main Jul 13, 2021
@glemaitre
Copy link
Member

Thanks @jmloyola

@jmloyola jmloyola deleted the fix_numpydoc_oneclasssvm branch July 13, 2021 17:24
ivannz added a commit to ivannz/scikit-learn that referenced this pull request Jul 23, 2021
@ivannz
migrate svdd code style to Black (scikit-learn#18948)
b379ea6 
ensure SVDD passes numpydoc validation (scikit-learn#20463)
check for svdd in `test_sparse.py:check_svm_model_equal` to avoid calling `.predict_proba`
@jmloyola jmloyola mentioned this pull request Aug 26, 2021
Merged
ivannz added a commit to ivannz/scikit-learn that referenced this pull request Nov 10, 2021
@ivannz
migrate svdd code style to Black (scikit-learn#18948)
bbedeef 
ensure SVDD passes numpydoc validation (scikit-learn#20463)
check for svdd in `test_sparse.py:check_svm_model_equal` to avoid calling `.predict_proba`
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
@jmloyola @glemaitre @samronsin
DOC Ensures that OneClassSVM passes numpydoc validation (scikit-learn…
7405056 
…#20463)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
ivannz added a commit to ivannz/scikit-learn that referenced this pull request May 15, 2022
@ivannz
migrate svdd code style to Black (scikit-learn#18948)
e192ef4 
ensure SVDD passes numpydoc validation (scikit-learn#20463)
check for svdd in `test_sparse.py:check_svm_model_equal` to avoid calling `.predict_proba`
ivannz added a commit to ivannz/scikit-learn that referenced this pull request Jun 14, 2022
@ivannz
migrate svdd code style to Black (scikit-learn#18948)
5642108 
ensure SVDD passes numpydoc validation (scikit-learn#20463)
check for svdd in `test_sparse.py:check_svm_model_equal` to avoid calling `.predict_proba`
ivannz added a commit to ivannz/scikit-learn that referenced this pull request Aug 29, 2022
@ivannz
migrate svdd code style to Black (scikit-learn#18948)
f346042 
ensure SVDD passes numpydoc validation (scikit-learn#20463)
check for svdd in `test_sparse.py:check_svm_model_equal` to avoid calling `.predict_proba`
ivannz added a commit to ivannz/scikit-learn that referenced this pull request Sep 5, 2022
@ivannz
migrate svdd code style to Black (scikit-learn#18948)
b0f4926 
ensure SVDD passes numpydoc validation (scikit-learn#20463)
check for svdd in `test_sparse.py:check_svm_model_equal` to avoid calling `.predict_proba`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glemaitre glemaitre glemaitre requested changes

@amueller amueller amueller left review comments

Assignees
No one assigned
Labels
Documentation module:svm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jmloyola @glemaitre @amueller