Skip to content

Comments

Enforce some pydocstyle lints with flake8-docstrings & docstring fixes in testing/#7603

Merged
bluetech merged 2 commits intopytest-dev:masterfrom
bluetech:flake8-docstrings
Aug 4, 2020
Merged

Enforce some pydocstyle lints with flake8-docstrings & docstring fixes in testing/#7603
bluetech merged 2 commits intopytest-dev:masterfrom
bluetech:flake8-docstrings

Conversation

@bluetech
Copy link
Member

@bluetech bluetech commented Aug 1, 2020

As discussed in #7471, we now have a preferred docstring style. While I was initially reluctant to add lints for the style we settled on, based on past experience, I tried pydocstyle again and it was quite useful, and I changed my mind. So this PR adds flake8-docstrings to our pre-commit setup.

The previous PR #7510 fixed docstring issues in src/. Since the lint works on the entire code, this PR also has a commit to fix issues in testing/.

tox.ini Outdated

[flake8]
max-line-length = 120
select = E,F,W,C,TYP,D200,D201,D206,D207,D208,D210,D211,D212,D214,D215,D300,D301,D403,D404
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pydocstyle has an extensive list of error codes. Most of them we don't want, and some have too many false-positives. So I opted to only select the error codes we want instead of excluding those we don't want.

I'm not sure if this is the best way to achieve this -- suggestions welcome.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general, extend-ignore is preferred over select

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, updated to use extend-ignore.

@bluetech

This comment has been minimized.

@bluetech bluetech closed this Aug 1, 2020
@bluetech bluetech reopened this Aug 1, 2020
@bluetech bluetech force-pushed the flake8-docstrings branch from de82ee6 to 3ed7a98 Compare August 1, 2020 15:24
@nicoddemus
Copy link
Member

So this PR adds flake8-docstrings to our pre-commit setup.

Does it fix the code for you automatically (like black), or does it just complain about it?

@bluetech
Copy link
Member Author

bluetech commented Aug 1, 2020

Does it fix the code for you automatically (like black), or does it just complain about it?

It just complains.

@nicoddemus
Copy link
Member

Gotcha!

In preparation for enforcing some docstring lints.
There are some ones we *would* like to enforce, like
    D401 First line should be in imperative mood
but have too many false positives, so I left them out.
@bluetech bluetech force-pushed the flake8-docstrings branch from 3ed7a98 to 9a18b57 Compare August 3, 2020 07:23
@bluetech bluetech merged commit 0dd5e16 into pytest-dev:master Aug 4, 2020
@bluetech bluetech deleted the flake8-docstrings branch August 24, 2020 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants