Additional flake8 plugins we'd like to use

[dgw]

I have a list of flake8 plugins I'd like to add after we release 7.0:

• flake8-future-import for easier enforcement of the required __future__ imports [Unignore more style checks #1561]
• flake8-import-order to check import sorting (kind of redundant with the existing isort project in isort: add isort to the QA tools #1752, but has the benefit of being integrated with our current checks; in-progress by me, @dgw, in a personal branch named flake8-import-order done by @half-duplex in Add flake8-import-order #1864)
• flake8-unused-arguments to do what it says on the tin (we can always ignore specific instances for e.g. backward compatibility with # noqa: U100 comments)
• flake8-comprehensions for sanity checks—these were some of the DeepSource warnings that I thought were actually worth looking at from [POC] all: fix deepsource checks #2016
• [ ] flake8-quotes to push us toward using single-quotes except when the string contains them (my own preference) (quality: add flake8-quotes #2322) Removed/NAK as the plugin can't be configured to enforce the desired style choices.
• flake8-requirements for core and plugins (as we extract stuff into separate packages), to make sure dependencies stay sane
• flake8-string-format to check .format() calls—though it mightn't be maintained any more? and/or there could be places where we intentionally break the conventions for stuff like pluralizing messages
• flake8-type-checking to guard against runtime overhead from importing modules that only appear in type annotations (implement flake8-type-checking #2300)
• flake8-docstrings to enforce blank lines (or not) around docstrings, and flag things that aren't documented but should be
  • Previously attempted in quality: Docstring cleanup for pydocstyle prep #1978, and it forced changes to a lot of things that made our docs (subjectively) worse. We'll need to fine-tune the selected rules in a future attempt.
• flake8-rst-docstrings to lint reStructuredText docstrings, keeping our docs more consistent

There are probably more plugins that would be useful, which anyone may suggest here. I'll modify this checklist as needed.

[Exirel]

I tend to completely ignore the "unused argument" warning in my code: sometimes, you need that argument in your interface, but not every implementation will need it, or even maybe most of them won't. It's OK, it's part of the life of an interface and its implementations.

I think there are better way to improve the code quality than checking that, and adding # noqa: U100 all over the place is not really my cup of tea.

My suggestion: we are all done for 7.1 as far as I'm concerned.

[dgw]

I still have a WIP branch for flake8-docstrings, and I don't intend to ship 7.1 without finishing it.

Unused args, fine, that decision can wait until after 7.1 and we check how many # noqas we'd need.

[half-duplex]

aaaaaaaaaaaaa
You should comment on issues with draft stuff you've worked on, cause I just spent most of the evening on it again.
Though yours looks like a challenging rebase anyway by now

[dgw]

I did self-assign this issue… But if you're considerably further along than mine (and I'm sure you are), don't worry about it. Especially now that I know someone else is working on it, I'm not gonna go finish mine and open a PR to snipe you.

[Exirel]

I'm looking at your branch @half-duplex and I really don't like this syntax:

"""
Synopsis of multi-line docstring.

Long description.
"""

I don't see the need for that change.

Also, I don't see the point of adding short docstring just for the sake of having a docstring.

[Exirel]

Also, if you want to flag stuffs without docstring: use pylint.

[half-duplex]

I'm looking at your branch @half-duplex and I really don't like this syntax

I wasn't going to break """Synopsis lines, but dgw's branch added that warning.

Also, I don't see the point of adding short docstring just for the sake of having a docstring.

Options: Disable missing docstring warning (ehh). noqa them (ehh). Or add a short, dubiously-useful docstring that can be filled out as needed (eh).

pydocstyle also doesn't seem to find superclass stub method docstrings, so I'm not sure how to deal with that. Right now I'm adding noqas as PyCQA/pydocstyle#309 mentions, since duplicating docstrings, breaking docs and introspection with """Override method.""", or adding a dependency and ignoring all checks for @override methods all seem worse.

[dgw]

I wasn't going to break """Synopsis lines, but dgw's branch added that warning.

It's a WIP, and shouldn't be treated as gospel. I don't like it either, and would have changed it myself before PR'ing.

[Exirel]

Yeah, it seems to me that what we want for now is just the pylint report.

When improving the code quality, it's often best to do that in 3 steps:

1. acknowledge the problem with reports that tell you what's wrong but don't prevent any build/deployment - that's what pylint will do in that case
2. focus on fixing the problems up to the point where you can put a strict rule, one type of problems at a time
3. enforce one specific rule and make it mandatory, making any build/deployment impossible if the code doesn't follow this new rule

Flake 8 is best at step 3, because we rely on it to validate the code before merge & packaging. Pylint is best for 1 & 2 as it generate a report you can use from time to time to know what's wrong and what you can fix.

[dgw]

I'm going to leave the rest of this stuff for 8.0. We put in a ton of doc/docstring work for 7.1, and I'd rather go over things again to fix warnings after we remove the deprecated stuff that's due for deletion in 8.0. 🙃

[dgw]

In case my closing note for #1978 wasn't clear enough, the target approach for docstrings at this moment is going to be those first 2 steps @Exirel mentioned. We'll probably want to avoid monolithic PRs for any of these, which means I fully expect the milestone to float along as we decide stuff is ready to ship and move to the next version.

[Exirel]

I've added flake8-quotes in #2322.

[dgw]

Because this won't affect the software in any way, I've decided it needn't be part of any milestone. "Tracking" and "Long-term Planning" labels both indicate it's more of a wishlist, not part of any specific roadmap.