Enforce basic PEP8 style with a pre-commit git hook

[peterjc]

See discussion on #406 and on the mailing list (e.g. http://mailman.open-bio.org/pipermail/biopython-dev/2014-October/020741.html ) where @cbrueffer proposed adopting this pre-commit git hook: https://github.com/cbrueffer/pep8-git-hook

I've been using this on Mac and Linux and it works very nicely.

Before adopting this we need to test it under Windows, where we need to know if it works, safely has no effect, or causes problems.

[peterjc]

I've since learnt that we can't enforce a git hook within the repository itself (which surprised me as things like .gitignore work that way). All we can do is document this (e.g. some projects bundle their pre-commit hook as a script in the repository and include a make command to install it).

[peterjc]

Comment from @MarkusPiotrowski on the mailing list:

Regarding Windows: I'm using Git for Windows and flake8 as pre-commit
hook which can easily be setup by flake8 itself:
$ flake8 --install-hook=git

http://lists.open-bio.org/pipermail/biopython-dev/2016-July/021468.html

This is very simple to install, although likely to be quite verbose and complain about many things already in Biopython (like our legacy mixed case module names).

[peterjc]

Thinking about this again, I think we should do something like this:

1. Create new file called Scripts/git_pre_commit_hook.py or similar (it could be written in bash?) within our repository
2. Provide instructions to set this up as a your pre-commit hook, cd biopython and run ln -s Scripts/git_pre_commit_hook.py .git/hooks/pre-commit

The new script would ideally cover not just the command line tool pep8, but also flake8, pydocstyle (for PEP257 docstrings) and eventually rst-lint as well - covering the same core set of checks as our TravisCI/Tox setup. Indeed, depending on how we resolve #883, the TravisCI/Tox setup might be implemented by calling the same script but with an option to check all the files not just the changed files?

[peterjc]

Work in progress here which runs flake8 etc and rst-lint (planned for the TravisCI/Tox setup soon):

https://github.com/peterjc/biopython/blob/pre_commit_linting/Scripts/git_pre_commit_hook.py

[peterjc]

I've updated the hook on my branch which runs flake8 and rst-lint to use the same configuration as our TravisCI/Tox setup - i.e. read the .flake8 and .pydocstyle files:

https://github.com/peterjc/biopython/blob/pre_commit_linting/Scripts/git_pre_commit_hook.py

This makes it much simpler to explain to a new-comer what is going on with the TravisCI tests vs the git hook, which is a plus.

I still like the idea of the git hook being stricter, but making them the same seems more practical. And over time we'll hopefully cleanup the back-log of style violations anyway.

[cbrueffer]

Nice, this is quite a step forward from the previous hook! I think whether TravisCI/local check have the same settings or local is stricter doesn't make much of a difference, so the current choice seems fine.

[peterjc]

I've been thinking (now that we run pydocstyle via its flake8 plugin) that using the flake8 hook might be simpler, although its default settings are not suitable so we'd need to recommend something like:

$ pip install flake8 flake8-docstrings flake8-blind-except
$ flake8 --install-hook git
$ git config --bool flake8.strict true

Personally, I set the configuration globally rather than just on the current repository. See: http://flake8.pycqa.org/en/latest/user/using-hooks.html

The downside is this would only check the Python files, and not the *.rst files or anything else. However, we don't have to maintain our own custom script - which is a big plus.

[peterjc]

As per my last comment, I was hoping we might be able to use the default flake8 pre-commit hook. However, right now that does not work well with per-folder .flake8 settings files: https://gitlab.com/pycqa/flake8/issues/360

Update: See flake8 feature request https://gitlab.com/pycqa/flake8/issues/347 to support directory/file level ignores in project configuration file

[peterjc]

My comment here #2014 (comment) was probably better on the original issue.

[peterjc]

Flake8 pre-commit hook documented as of #2075.