Run linters and formatters via pre-commit

[JoshKarpel]

Hi again!

While working on #794 I had a few flake8 linting errors locally that I didn't catch before pushing and had to push again to fix. On other projects I've found pre-commit very helpful for preventing these errors by forcing linters and formatters to run before committing. This PR adds a pre-commit configuration which includes the existing flake8 and isort checks, plus some basic formatters and linters suggested by pre-commit. I've updated CONTRIBUTING.md to include instructions for setting up pre-commit locally and installing the hooks.

In this initial take on the PR I've left flake8 and isort running through tox as well, but there's a CI service associated with pre-commit at https://pre-commit.ci/ that I like (free for open source projects). It runs pre-commit checks on PRs and sends a PR every week to update the pre-commit check dependencies. It's also possible to run the pre-commit checks in CircleCI, example config here. I'd recommend removing the tox configuration for running flake8 and isort and running them through pre-commit instead, either through pre-commit.ci or CircleCI - your preference, assuming you're interested in this :)

[JoshKarpel]

Seems like the parser requires the trailing space on these lines, which the end-of-line fixer I added in the pre-commit config wanted to remove from the multiline string. Possibly a separate bug in the parser? I don't know enough about the openmetrics format to say.

[brian-brazil]

The parser is correct, also changing things like this makes it harder to manually run these tests via copy&page. They're written this way for a reason.

[JoshKarpel]

Good to know - lemme revert this part and remove the end-of-line fixer as a quick fix. There are ways to ignore particular files, but I don't love doing that, so it seems like that particular formatter needs some more thought.

[brian-brazil]

The parser is correct, also changing things like this makes it harder to manually run these tests via copy&page. They're written this way for a reason.

[brian-brazil]

You are changing this test, this trailing space is meant to be here.

[csmarchbanks]

Thanks for the PR!

Personally, I am not a huge fan of pre-commit hooks as they add more overhead than I would like for some changes, or when I just want to push my progress somewhere. Yes --no-verify is an option, but I have still found them a bit jarring.

I would like to leave this open for a bit though, if other devs think it is helpful I am not opposed to merging it.

[JoshKarpel]

Personally, I am not a huge fan of pre-commit hooks as they add more overhead than I would like for some changes, or when I just want to push my progress somewhere. Yes --no-verify is an option, but I have still found them a bit jarring.

Fair point! However, this set of checks is pretty fast.

At least on my computer, running these pre-commit checks on all files (which is the absolute maximum, since during a commit they'll only run on files in the diff) takes ~4 seconds

❯ time pre-commit run --all
check for added large files..............................................Passed
check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check for case conflicts.................................................Passed
check docstring is first.................................................Passed
check for merge conflicts................................................Passed
check toml...........................................(no files to check)Skipped
check yaml...............................................................Passed
debug statements (python)................................................Passed
fix end of files.........................................................Passed
flake8...................................................................Passed
isort....................................................................Passed

real    0m4.080s
user    0m8.024s
sys     0m1.443s

I'm running through WSL so I suspect this is longer than average due to how slow WSL is with filesystem operations. In my experience, those few seconds are worth it to not have to wait for CI to fail and do a whole extra commit + push to fix the typos.