MNT Update sklearn.externals._lobpcg.lobpcg docstring according to https://github.com/scipy/scipy/pull/16432 #23597
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14c4b55 to
32af96f
Compare
thomasjpfan
reviewed
Jun 12, 2022
…dule doesn't work on some systems Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
|
But we should already ignore it, isn't it: scikit-learn/sklearn/tests/test_docstrings.py Lines 156 to 166 in 2f787f4 |
Yes, it should be ignored. But for some unknown reason the test runs and fails on my local machine. Since there is already another function of the externals module listed in |
|
The Scipy devs fixed the docstring themselves scipy/scipy#16432. I updated this PR accordingly. |
thomasjpfan
approved these changes
Jun 21, 2022
|
Thanks @Diadochokinetic Merging |
…ipy/scipy#16432 (scikit-learn#23597) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
glemaitre
pushed a commit
to glemaitre/scikit-learn
that referenced
this pull request
Aug 4, 2022
…ipy/scipy#16432 (scikit-learn#23597) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
glemaitre
pushed a commit
that referenced
this pull request
Aug 5, 2022
…ipy/scipy#16432 (#23597) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
Fixes #23596
What does this implement/fix? Explain your changes.
pytest -vl sklearn/tests/test_docstrings.py -k test_docstringthrows a ValueError forsklearn.externals._lobpcg.lobpcg. A quick fix would be to add the function to FUNCTION_DOCSTRING_IGNORE_LIST and simply exclude it from the test. A proper fix would be to actually fix the errors on the docstring:E # Errors
E
E - GL03: Double line break found; please use only one blank line to separate sections or paragraphs, and do not leave blank lines at the end of docstrings
E - SS03: Summary does not end with a period
E - PR08: Parameter "Y" description should start with a capital letter
E - RT05: Return value description should finish with "."
Edit: Fixing the docstring wasn't that hard. Therefore I suggest to go for that solution.
Any other comments?
I started by simply adding the function to the ignore list. Then I fixed the docstring and removed the function from the ignore list. Now the tests run successfully.
EDIT: The externals module should actually be excluded from docstring tests. It is unknown, why it happens on my local machine. Since there is already a function from the externals module listet in FUNCTION_DOCSTRING_IGNORE_LIST, this probably happened before. Therefore simply adding it to the ignored list is preferred.