This repository has been archived by the owner on Nov 3, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 187
add support for sphinx-style parameters to D417 #595
Merged
sigmavirus24
merged 3 commits into
PyCQA:master
from
benji-york:benji/add-sphinx-style-parameters
Sep 22, 2023
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
"""Unit tests for Sphinx-style parameter documentation rules. | ||
|
||
Use tox or pytest to run the test suite. | ||
""" | ||
from pydocstyle.checker import ConventionChecker | ||
import textwrap | ||
import pytest | ||
|
||
SPHINX_ARGS_REGEX = ConventionChecker.SPHINX_ARGS_REGEX | ||
|
||
def test_parameter_found(): | ||
"""The regex matches a line with a parameter definition.""" | ||
line = " :param x: Lorem ipsum dolor sit amet\n" | ||
assert SPHINX_ARGS_REGEX.match(line) is not None | ||
|
||
|
||
def test_parameter_name_extracted(): | ||
"""The first match group is the parameter name.""" | ||
line = " :param foo: Lorem ipsum dolor sit amet\n" | ||
assert SPHINX_ARGS_REGEX.match(line).group(1) == "foo" | ||
|
||
|
||
def test_finding_params(): | ||
"""Sphinx-style parameter names are found.""" | ||
docstring = """A description of a great function. | ||
|
||
:param foo: Lorem ipsum dolor sit amet | ||
:param bar: Ut enim ad minim veniam | ||
""" | ||
|
||
lines = docstring.splitlines(keepends=True) | ||
assert ConventionChecker._find_sphinx_params(lines) == ['foo', 'bar'] | ||
|
||
|
||
def test_missing_params(): | ||
"""Missing parameters are reported.""" | ||
source = textwrap.dedent('''\ | ||
def thing(foo, bar, baz): | ||
"""Do great things. | ||
|
||
:param foo: Lorem ipsum dolor sit amet | ||
:param baz: Ut enim ad minim veniam | ||
""" | ||
pass | ||
''') | ||
errors = ConventionChecker().check_source(source, '<test>') | ||
for error in errors: | ||
if error.code == "D417": | ||
break | ||
else: | ||
pytest.fail('did not find D417 error') | ||
|
||
assert error.parameters == ('bar', 'thing') | ||
assert error.message == ( | ||
"D417: Missing argument descriptions in the docstring (argument(s) bar are" | ||
" missing descriptions in 'thing' docstring)") | ||
|
||
|
||
def test_missing_description(): | ||
"""A parameter is considered missing if it has no description.""" | ||
source = textwrap.dedent('''\ | ||
def thing(foo, bar, baz): | ||
"""Do great things. | ||
|
||
:param foo: Lorem ipsum dolor sit amet | ||
:param bar: | ||
:param baz: Ut enim ad minim veniam | ||
""" | ||
pass | ||
''') | ||
errors = ConventionChecker().check_source(source, '<test>') | ||
for error in errors: | ||
if error.code == "D417": | ||
break | ||
else: | ||
pytest.fail('did not find D417 error') | ||
|
||
assert error.parameters == ('bar', 'thing') | ||
assert error.message == ( | ||
"D417: Missing argument descriptions in the docstring (argument(s) bar are" | ||
" missing descriptions in 'thing' docstring)") |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benji-york could you please move this to the latest version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benji-york any objections if I have a go at updating this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd be great, @tomghc!
(Sorry, I let this fall off my radar. If I can be of any service, let me know.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benji-york Hey, I was looking to help finish updating this to the current version so it could be pushed. Is there any way I could aid in development?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy for anyone to do anything on this. I'm not sure what @samj1912 means above by "move this to the latest version". Perhaps rebase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kicked off an automated rebase, but the workflows are awaiting maintainer approval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The automated rebase was successful. All tests passed. Now the review status has been reset, so this needs another review by a maintainer and if still acceptable (which it should be because nothing has changed), it needs merging by a maintainer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds great! Apologies for any inconvenience that I may have caused. I appreciate the work you put in!