Skip to content

DOC Ensures that sklearn.datasets._base.load_breast_cancer passes numpydoc validation#22346

Merged
glemaitre merged 5 commits intoscikit-learn:mainfrom
DennisOsei:numpydoc-validation
Feb 1, 2022
Merged

DOC Ensures that sklearn.datasets._base.load_breast_cancer passes numpydoc validation#22346
glemaitre merged 5 commits intoscikit-learn:mainfrom
DennisOsei:numpydoc-validation

Conversation

@DennisOsei
Copy link
Copy Markdown
Contributor

@DennisOsei DennisOsei commented Jan 31, 2022

Reference Issues/PRs

Addresses #21350

What does this implement/fix? Explain your changes.

  1. Removed sklearn.datasets._base.load_breast_cancer from FUNCTION_DOCSTRING_IGNORE_LIST
  2. Added spaces before and after colon of return values in sklearn.datasets._base.load_breast_cancer.
  3. Redefined the description of return value of (data, target)
  4. Removed trailing whitespace from description of return value of (data, target)
  5. Moved the link of UCI ML Breast Cancer Wisconsin (Diagnostic) dataset to below the breast cancer dataset table.

Any other comments?

@DennisOsei
Copy link
Copy Markdown
Contributor Author

DennisOsei commented Jan 31, 2022

The pytest tests passed this time.

@ArturoAmorQ
Copy link
Copy Markdown
Member

ArturoAmorQ commented Jan 31, 2022

You seem to have merge conflicts, My guess is that you are using the same branch as in #22266, without removing said branch from the remote first.

@glemaitre glemaitre self-requested a review January 31, 2022 10:32
@glemaitre
Copy link
Copy Markdown
Member

glemaitre commented Jan 31, 2022

It is possible that we merge some similar PRs that remove some of the flagged functions. I merged main into your PR to solve the conflict. The tests were passing locally so let's see if the CIs will be fine as well.

If yes, I will merge this PR.

@ArturoAmorQ
Copy link
Copy Markdown
Member

ArturoAmorQ commented Jan 31, 2022

If yes, I will merge this PR.

My suggestions from #22266 got lost by closing that PR and had not yet been addressed. I am re-posting them here

ArturoAmorQ
ArturoAmorQ reviewed Jan 31, 2022
View reviewed changes
@DennisOsei
Copy link
Copy Markdown
Contributor Author

DennisOsei commented Jan 31, 2022

@ArturoAmorQ , @glemaitre am I supposed to do something right now? My branch hasn't been merged.

@glemaitre @ArturoAmorQ
Update sklearn/datasets/_base.py
3603e53 
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
@glemaitre
Copy link
Copy Markdown
Member

glemaitre commented Feb 1, 2022

I applied the suggestion of @ArturoAmorQ and I will merge when the CIs are green

@glemaitre glemaitre merged commit a793c1f into scikit-learn:main Feb 1, 2022
@glemaitre
Copy link
Copy Markdown
Member

glemaitre commented Feb 1, 2022

Thanks @DennisOsei all good.

@DennisOsei DennisOsei deleted the numpydoc-validation branch February 2, 2022 05:29
@DennisOsei
Copy link
Copy Markdown
Contributor Author

DennisOsei commented Feb 2, 2022

Thanks @glemaitre . Also it seems that the old PR is the one showing up in functions to update instead of this one. Is there anything that can be done to fix this?

@glemaitre glemaitre mentioned this pull request Feb 8, 2022
Closed
@glemaitre
Copy link
Copy Markdown
Member

glemaitre commented Feb 8, 2022

I updated it. Thanks for pointing this out.

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

@ArturoAmorQ ArturoAmorQ ArturoAmorQ left review comments

Assignees

No one assigned

Labels

Documentation module:datasets No Changelog Needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DennisOsei @ArturoAmorQ @glemaitre