The Wayback Machine - https://web.archive.org/web/20250524073653/https://github.com/scikit-learn/scikit-learn/pull/20713
Skip to content

DOC Ensures MultiOutput* and RegressorChain passes numpydoc validation #20713

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 29 commits into from
Sep 1, 2021

Conversation

reshamas
Copy link
Member

@reshamas reshamas commented Aug 8, 2021

Reference Issues/PRs

Addresses #20308

What does this implement/fix? Explain your changes.

Fixes formatting to pass numpy validation tests for class MultiOutput* and RegressionChain

Any other comments?

#DataUmbrella LATAM sprint

@reshamas reshamas changed the title DOC ensure ClassifierChain passes numpydoc validation DOC Ensures ClassifierChain passes numpydoc validation Aug 9, 2021
@reshamas
Copy link
Member Author

reshamas commented Aug 9, 2021

@glemaitre
I think I see where the error is. In this rendered file,
https://147552-843222-gh.circle-artifacts.com/0/doc/modules/generated/sklearn.multioutput.MultiOutputClassifier.html

It gets rendered as:
property predict_proba

But, I am not sure where/how to fix it. It's line 412 here:

    @property
    def predict_proba(self):
        """Probability estimates.
        Returns prediction probabilities for each class of each output.

        This method will raise a ``ValueError`` if any of the
        estimators do not have ``predict_proba``.

@glemaitre
Copy link
Member

The property bad rendering should not be the issue. It is indeed not a nice rendering but the introduction of available_if instead of the property will solve this rendering issue (e.g. #20667)

@reshamas
Copy link
Member Author

reshamas commented Aug 9, 2021

Should I be commenting out all 4 of these in test_docstrings.py? Currently, I have just commented out ClassifierChain.

__all__ = [
    "MultiOutputRegressor",
    "MultiOutputClassifier",
    "ClassifierChain",
    "RegressorChain",
]

@glemaitre
Copy link
Member

Yes if you are modifying the docstring of the other estimators, you can uncomment. I can update the title.

@glemaitre glemaitre self-requested a review August 31, 2021 17:02
@glemaitre glemaitre changed the title DOC Ensures ClassifierChain passes numpydoc validation DOC Ensures MultiOutput* and RegressorChain passes numpydoc validation Sep 1, 2021
@glemaitre
Copy link
Member

Since I merged yesterday the docstring for ClassifierChain, I merged main in the branch and uncommented for the MultiOutput* estimator and RegressorChain. It was passing locally so I think that everything is fine. Let see if the CIs will agree.

@glemaitre glemaitre self-requested a review September 1, 2021 11:54
@glemaitre glemaitre merged commit 907c093 into scikit-learn:main Sep 1, 2021
@glemaitre
Copy link
Member

Thanks @reshamas

@reshamas reshamas deleted the ClassifierChain branch September 1, 2021 13:06
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
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.

2 participants