switch to pre-commit

[akaszynski]

tl;dr

Let's use pre-commit to simplify our lives and make it easier and more fun to contribute.

Problem

We're quite strict about our code standards, and that's great! We're writing consistent, readable code that many people feel comfortable contributing to. The problem with our existing approach is it's:

• not cross-platform (windows doesn't natively support makefiles)
• not automatic
• scattered across several makefile sections
• not intuitive

This makes it really, really annoying to ensure commits meet coding standards as one has to fire off one or more make commands and remember to do it.

Proposed Solution

This PR suggests we use pre-commit as done in pandas/.pre-commit-config.yaml to:

• provide a cross-platform solution
• automate it with a simple to install pre-commit hook
• run automatically (and quickly) after each and every commit

pre-commit works on all major OSes, and is installed with:

pip install pre-commit

and then can be (manually) used with:

pre-commit run --all-files

or "installed" as a pre-commit hook with:

pre-commit install

where it will be run after every commit:

(py38) alex@enterprise:~/python/pyvista$ git commit -am "implement mypy and update yml"
black....................................................................Passed
isort....................................................................Passed
flake8...................................................................Passed
codespell................................................................Passed
pydocstyle...........................................(no files to check)Skipped
mypy.................................................(no files to check)Skipped

It only checks the files staged for the commit and is therefore wicked fast, something we want if we want contributors to want to use this tool. Best part is if something needs to be autoformatted by black or isort, you run the tool, it modifies the files and tells you it failed, and then you simply commit again.

It's super automatic and means we won't have to keep coming back to failed PRs because we had a character of whitespace on L2087.

Implementations

This PR removes:

• The requirements_style.txt and requirements_static.txt and these versions are now tracked in .pre-commit-config.yaml.
• Several sections have been removed from the Makefile. "There should be one-- and preferably only one --obvious way to do it."

[codecov]

Codecov Report

Merging #2392 (a69e1eb) into main (c8be2c4) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2392      +/-   ##
==========================================
- Coverage   93.60%   93.53%   -0.07%     
==========================================
  Files          74       74              
  Lines       15831    15831              
==========================================
- Hits        14818    14808      -10     
- Misses       1013     1023      +10     

[tkoyama010]

Thanks for this. LGTM with a small fix.

[adeak]

Thanks for this, I've been meaning to do that for a while but never got around to it.

[dcbr]

Really great addition which will save a lot of time :)

I haven't tested it locally, but what happens if multiple formatting steps would fail (e.g. both black and isort have to modify files)? Do you have to commit again for each separate step or are all changes performed at once, requiring only a single new commit attempt afterwards?
Also does the error message explicitly say to try and commit again, because otherwise it might be quite confusing as to why commiting failed exactly?

[MatthewFlamm]

if multiple formatting steps would fail (e.g. both black and isort have to modify files)? Do you have to commit again for each separate step or are all changes performed at once, requiring only a single new commit attempt afterwards?

The attempted commit is aborted and changes are made to the local workspace, in the case of black and isort. You have to git add the changes and then commit again.

One additional benefit of pre-commit is that it runs against the code being committed, which prevents errors when you forget to stage changes.

[akaszynski]

Added in a few other pre-commit checks:

• Disable committing to main
• Flag introduction of breakpoints (I've done that)
• Check merge conflicts
• Validate github yml schema (this will save us from failed runs for boneheaded mistakes)