Flake8 plugins in pre-commit

[CaedenPH]

Feature description

Certain flake8 plugins increase code quality.
Two of these plugins have been implemented so far, pep8-naming and flake8-builtins.
What other plugins should be implemented?

Would you like to work on this feature?

• Yes, I want to work on this feature!

[CaedenPH]

@cclauss @dhruvmanila

[cclauss]

Maybe docstrings? https://github.com/PyCQA?q=flake8- But I am sure a ton of existing files do not have docstrings.

Also like flake8-2020, flake8-bugbear, flake8-comprehensions, flake8-return, flake8-simplify but... we need to be careful not to go overboard and make contributors too angry to fix all the nits.

[CaedenPH]

Personally I would go for flake8-bugbear, docstrings would be a hell of a jump

[cclauss]

Simplify and comprehensions come up with some quite useful suggestions.

Return is helpful for beginners but they have a few errors that I do not agree with.

[CaedenPH]

Can I make a pull request implementing flake8-bugbear?

[Cjkjvfnby]

Plugins that are useful for me are:

• flake8-eradicate remove commented-out code, quite useful to keep the repo in shape.
• flake8-pytest-style makes all tests look consistent

My personal list is quite huge, but I find it too strict for the project with many contributors.
https://github.com/Cjkjvfnby/project_template/blob/master/%7B%7Bcookiecutter.folder_name%7D%7D/.pre-commit-config.yaml

The thing I learned is that things are constantly broken because of library updates, so I prefer to use fixed versions.
Especially because I use pre-commit as a GitHub action.

If you wold like, I could contribute adding this plugins.

[Cjkjvfnby]

To run flake8 with plugins you need to install them first.
This line in the contribution guide don't mention this.

Since flake8 is already included in pre-commit run --all-files ... , this string might be just removed.

All submissions will need to pass the test flake8 . --ignore=E203,W503 --max-line-length=88 before they will be accepted so if possible, try this test locally on your Python file(s) before submitting your pull request.

[cclauss]

For this repo, I would say no to both eradicate and pytest style.

• Eradicate -- This is not a production code repo -- instead, it is a center for learning and sharing so commented code might be quite acceptable and removing comments without human intervention sounds scary to me.
• Pytest -- We focus a lot on doctest and have very few pytests. Of those few that we do have, we do not want to set the bar for contribution to be much higher than pass/fail.

[cclauss]

Most of our contributors are first-time-contributors and almost none of them have pre-commit installed and running locally which is why we run pre-commit.ci but a pull request to fix README.md would be welcome.

[cclauss]

flake8-use-pathlib sounds awesome (although I have never seen it in action).

[Cjkjvfnby]

Maybe docstrings? https://github.com/PyCQA?q=flake8- But I am sure a ton of existing files do not have docstrings.

Flake is pretty flexible with ignoring files with specific errors.
You could suppress specific errors for files in the flake config. It looks a bit bulky but allows you to avoid touching the code.
https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-per-file-ignores

I use this plugin for my code and exclude test files from it.

per-file-ignores =
    # No docs and annotation required for tests
    tests/*.py:    D10

[cclauss]

A ton means a ton -- take a scroll.

[Cjkjvfnby]

flake8-use-pathlib sounds awesome (although I have never seen it in action).

I have some concerns about it. This code looks super unusual to me.

with Path(filename).open() as f:

A ton means a ton -- take a scroll.

I could extract errors from the flake8 output and just build a flake8 setting from it. So the new contributions will have docstrings.

[cclauss]

OK. I will try out use-pathlib and see for myself.

If you want to do a PR on docstrings, that would be reviewed. Thx.

[Cjkjvfnby]

Sample PR on how the code should look to conform the flake8-docstrings. #7821

It would be nice to understand the check that we plan to keep and which one we could ignore.

Fixing all of them sounds boring. We could either ignore them or keep checking only for new files.
Maybe pre-commit.ci could just ignore existing errors and just force to fix them a person who is editing the files.

[Cjkjvfnby]

I'd say it is closed a bit too earlier. I haven't submitted the PR with annotation-docstring yet. I am planning to do it today in the evening, so no need to reopen it.

I have learned a lot of cool things from yours pre-commit hooks, things like yesqa are exactly what I need in my projects.

[cclauss]

I need to try to review the 150+ current open PRs so please hold off on adding more PRs until Tuesday, Nov 1st.

[Cjkjvfnby]

I have a free evening today, could I help with the review?