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 LinearRegression passes numpydoc validation #20369

Merged
merged 4 commits into from
Jun 28, 2021
Merged

DOC Ensures that LinearRegression passes numpydoc validation #20369

merged 4 commits into from
Jun 28, 2021

Conversation

gloriamacia
Copy link
Contributor

@gloriamacia gloriamacia commented Jun 26, 2021 •
edited
Loading

Reference Issues/PRs

Addresses #20308 for LinearRegression with my pair programming partner @caherrera-meli
What does this implement/fix? Explain your changes.

Added periods to docstring summaries and reorganized the classes docstring to follow the suggested order.
Any other comments?

#DataUmbrella sprint

@thomasjpfan thomasjpfan mentioned this pull request Jun 26, 2021
Closed
@glemaitre glemaitre changed the title DOC Ensures that LinearRegression passes numpydoc validation | Addresses #20308 DOC Ensures that LinearRegression passes numpydoc validation Jun 26, 2021
@gloriamacia
Copy link
Contributor Author

gloriamacia commented Jun 26, 2021 •
edited
Loading

@glemaitre We are getting this error "Linux pylatest_pip_openblas_pandas failed" no idea what it means but seems unrelated to the DOC change.

@glemaitre
Copy link
Member

So the test is no passing, you need to reformat the docstring. You need to reformat:

  • LinearRegression.fit:
    • PR09: Parameter "X" description should finish with "."
    • PR09: Parameter "y" description should finish with "."
    • PR09: Parameter "sample_weight" description should finish with "."
    • RT03: Return value has no description
  • LinearRegression.score:
    • SS06: Summary should fit in a single line

@glemaitre
Copy link
Member

The class is declared in the file sklearn/linear_model/_base.py

@gloriamacia
Copy link
Contributor Author

@glemaitre thanks a lot! We just realized numpydoc was not properly installed so tests were being skipped... we will fix it and re-submit.

@glemaitre
Copy link
Member

You can make the changes on your local branch, commit, and push your branch. Be aware that you don't have to open a new PR. Pushing the current PR will be synchronized in GitHub automatically.

@glemaitre
Copy link
Member

Since this is a little change, I merged my suggestion and I will merge the PR now. Thanks @gloriamacia

@glemaitre glemaitre merged commit a2ee56c into scikit-learn:main Jun 28, 2021
@gloriamacia gloriamacia deleted the linear_regression branch July 3, 2021 16:06
@gloriamacia
Copy link
Contributor Author

#DataUmbrella

samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
@gloriamacia @glemaitre @samronsin
DOC Ensures that LinearRegression passes numpydoc validation (scikit-…
066f950 
…learn#20369)

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 left review comments

Assignees
No one assigned
Labels
Documentation module:linear_model
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gloriamacia @glemaitre