Skip to content

DOC Ensures that RandomForestClassifier passes numpydoc validation #20383

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
Merged
glemaitre merged 10 commits into from
Jul 7, 2021
Merged

DOC Ensures that RandomForestClassifier passes numpydoc validation #20383

glemaitre merged 10 commits into from
Jul 7, 2021

Conversation

@alinealfa
Copy link
Contributor

@alinealfa alinealfa commented Jun 26, 2021

Reference Issues/PRs

Addresses #20308

What does this implement/fix? Explain your changes.

Fixes numpydoc errors on RandomForestClassifier
Forces decorate property to update doc for deprecated method

Any other comments?

#DataUmbrella #sprint
cc. @marenwestermann

@thomasjpfan thomasjpfan mentioned this pull request Jun 26, 2021
Closed
glemaitre
glemaitre reviewed Jun 26, 2021
View reviewed changes
@glemaitre
Copy link
Member

glemaitre commented Jun 26, 2021

#20385 has been merged. You can merge main into your branch to get the changes.

@glemaitre
Copy link
Member

glemaitre commented Jun 26, 2021

Uhm black is not happy with the formatting of _forest.py. Once the suggestion applied could you execute black on the python file.

@marenwestermann
Copy link
Member

marenwestermann commented Jun 26, 2021

We ran black after implementing the changes and merging main into our branch but there was no reformatting.

glemaitre
glemaitre reviewed Jun 26, 2021
View reviewed changes
@glemaitre
Copy link
Member

glemaitre commented Jun 26, 2021

Yep black is not complaining anymore. However, my suggestion is too long. I propose to break the line and flake8 should be fine.

@alinealfa alinealfa mentioned this pull request Jun 26, 2021
Merged
glemaitre
glemaitre reviewed Jun 28, 2021
View reviewed changes
@glemaitre glemaitre self-requested a review July 6, 2021 09:10
glemaitre
glemaitre approved these changes Jul 6, 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.

I took the liberty to merge main into your branch and change the docstring of feature_in_ to see if the tests are passing. If the CIs agree then we can merge it.

glemaitre
glemaitre reviewed Jul 7, 2021
View reviewed changes
@glemaitre glemaitre self-requested a review July 7, 2021 10:12
glemaitre
glemaitre reviewed Jul 7, 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.

I merge main into the branch to solve the CI doc failure.

@glemaitre glemaitre merged commit f2a6e10 into scikit-learn:main Jul 7, 2021
@glemaitre
Copy link
Member

glemaitre commented Jul 7, 2021

The CIs are green, let's merge. Thanks @alinealfa

thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Jul 7, 2021
@alinealfa @glemaitre @thomasjpfan
DOC Ensures that RandomForestClassifier passes numpydoc validation (s…
61775f7 
…cikit-learn#20383)

Co-authored-by: Guillaume Lemaitre <[email protected]>
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
@alinealfa @glemaitre @samronsin
DOC Ensures that RandomForestClassifier passes numpydoc validation (s…
1c7f9c7 
…cikit-learn#20383)

Co-authored-by: Guillaume Lemaitre <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@glemaitre glemaitre glemaitre approved these changes

Assignees

No one assigned

Labels

Documentation module:ensemble module:utils

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alinealfa @glemaitre @marenwestermann