Skip to content

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

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
merged 5 commits into from
Feb 1, 2022
Merged

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

merged 5 commits into from
Feb 1, 2022

Conversation

DennisOsei
Copy link
Contributor

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

The pytest tests passed this time.

@ArturoAmorQ
Copy link
Member

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
Member

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
Member

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

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

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
Member

Thanks @DennisOsei all good.

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

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
Member

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