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 RandomForestClassifier passes numpydoc validation #20383

Merged
merged 10 commits into from
Jul 7, 2021
Merged

DOC Ensures that RandomForestClassifier passes numpydoc validation #20383

merged 10 commits into from
Jul 7, 2021

Conversation

alinealfa
Copy link
Contributor

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
sklearn/ensemble/_forest.py Outdated Show resolved Hide resolved
sklearn/utils/deprecation.py Outdated Show resolved Hide resolved
sklearn/ensemble/_forest.py Outdated Show resolved Hide resolved
@glemaitre
Copy link
Member

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

@glemaitre
Copy link
Member

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

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

@glemaitre
Copy link
Member

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 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 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

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 <g.lemaitre58@gmail.com>
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 <g.lemaitre58@gmail.com>
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