NCAS-CMS / cfunits

A Python interface to UNIDATA’s UDUNITS-2 library with CF extensions:
http://ncas-cms.github.io/cfunits
MIT License
11 stars 8 forks source link

Back in black: auto-formatting and pre-commit hooks #21

Closed sadielbartholomew closed 3 years ago

sadielbartholomew commented 3 years ago

Set up the use of pre-commit hooks to lint any Python modules (only) in the codebase for validity, formatting and style issues so they are raised and can be amended before they can be committed, with an initial sweep of the codebase with these tools so all modules are up-to-standard by the same criteria to begin with.

This way the codebase is and should remain free of any such issues, though note I have left in (untouched) the test_style.py test in the test suite in order to confirm we abide by vanilla PEP8 via running the pycodestyle check (I would have added in black and/or flake8 checks in new methods to this test module, but it seems these are designed for the CLI only since, despite being Python libraries, from interactive inspection and searching online they seem to have made it as difficult as possible, and not documented how, to run the checks programmatically in Python itself :grimacing:).

The new linting tools used are:


Notes on pre-commit hooks

@davidhassell the details above aren't important (unless you are especially interested!) but do be aware that this PR sets up pre-commit hooks which means that, after it is merged, any attempt to make a commit locally will initiate some checks on files that have been changed and staged, the output of which looks similar to this:

$ git commit -m "Fix issues raised by flake8 excluding skip and potential bug"
[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to /home/sadie/.cache/pre-commit/patch1610396726.
Check python ast.........................................................Passed
Debug Statements (Python)................................................Passed
Fix End of Files.........................................................Passed
Trim Trailing Whitespace.................................................Passed
black....................................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

cfunits/units.py:768:17: F841 local variable '_units_since_reftime' is assigned to but never used

If they pass, the commit is made as standard, but if at least one check fails then the commit will not be made until any issues raised are fixed. Some feedback should be given to indicate what needs to be done.

Note you will need to install the tools, e.g. this does it:

$ pip install pre-commit
$ pip install black
$ pip install flake8

and additionally on any machine used to commit to the repo, first run:

$ pre-commit install

to process the pre-commit configuration file which install the hooks to enable those checks to run all the time (it seems to re-run on start-up so no need to running that one every day).

Tip: if (as I often do in) you want to make "dev" commits which may be squashed or discarded later, i.e. not intended as a formal commit that will be pushed, you can skip all those checks with the --no-verify option to git commit. Though I think we should try to avoid using that to skip the checks unless we know we won't be pushing the commit in question in the same form.