Code quality toolchain: black + pylint + mypy

[davidorme]

Background

I'm preparing to run some workshops on the core toolchains to allow us to focus RCS training on group wide knowledge gaps. This Q&A is to try and align what I'm thinking with RCS and the team, so I start from the right page.

I think of black + pylint + mypy as a coherent group of code quality tools that need to be applied to commits. My feeling is that they represent increasing severity of checking, so get applied at with different frequency and that there is a natural order to applying the tools.

1. black is an automatic code formatter. It applies an opinionated, hard set of formatting rules that convert input code to that common standard. It's quite trendy, but there is some pushback (including from Guido van Rossum!) and this post is a good explanation of why:

   https://luminousmen.com/post/my-unpopular-opinion-about-black-code-formatter

   Essentially, black moves code around on the page - where you put line breaks - and has a few stylistic rules (like having strings in double quotes), but it is brute force and doesn't allow for individual choice on readability. That said, with a new team and a new project, having a simple tool that makes us all use the same format seems neat.

   If as a team we seriously object to some feature of the autoformat, there might be a fallback with the darker (https://pypi.org/project/darker/) tool, which only formats new code in commits.

   black should be run before pylint because it catches and removes some of the things that pylint dislikes.

2. pylint is a linter, which is a very different beast from an autoformatter. It reports on a vast list of things that are (probably) wrong with your code and that you need to fix. These include some simple formatting issues that black would catch, but then runs a battery of testing to spot code issues, ranging from simple variable naming convention violations to likely structural problems.

   It also gives you a score on your code - which is good for feeling like you are making progress but can be initially upsetting:

    -----------------------------------
    Your code has been rated at 1.17/10
   

   Linting is run on a module level - it picks up things that are wrong in this individual code file - but not across modules. Most commits should be linted, but insisting on perfect lint scores on all commits is likely to be a barrier to development.

3. mypy checks that type hints exists for callable objects and then that code calling those objects obeys those type hints: that the arguments and return values are of the right types.

   It can be run on an individual module, and so could be run per-commit, but is of more value when run across a wider scope to catch typing consistency across modules and imports.

Questions

• Are we happy that all commits should be run through black? We can use Git pre-commit hooks to enforce this.
• When should we be running pylint? Is it common to have code development and then linting commits, rather than linting of every commit?
• When should we be running mypy? Is it common to have mypy commits to align typing at broader scopes?

[dalonsoa]

Hi all,

This is a really nice summary of tools to use. I would say that this stack is a must and starting with a fresh project will make things easy.

About the individual tools:

• Black: it is extremely useful. Granted that some stylistic choices might be a bit radical or odd-looking, but it tries its best to conform to the general Python style rules which I take they and a lot of thought behind. It rarely needs to be run manually. I have it configured such that it runs automatically whenever I save a file.

• Pylint: not used it myself much. We tend to use flake8. It might be just because of historic reasons rather than because of a thorough comparison of features, but it works really well in tandem with black. In any case, any of them will do a good job. It might be annoying but, again, it is there to ensure that the code conforms to some standard rules.

• mypy: this is incredibly useful, in my experience the most annoying of the lot, but the one that ensures best code in the long run. A pain to use in an old code, but should be pretty painless in brand new code.

I would like to the stack:

• isort: which sorts the imports statements in a standardised manner across the code.

• pre-commit: the crown jewel, which will run all of the above automatically (as well as other things) whenever you make a commit, preventing you to submit any code that does not conforms with the appropriate standards. Sound harsh. It is harsh. But it is worth it if we aim to have a profesional looking and sustainable code.

And now, the best thing: we have created a GitHub repository template using poetry and configured with all of the above, so manual setup should be minimal. It uses flake8, but it should be easy to adapt for Pylint if you prefer to go with that one.

I hope this helps to make a decision on what to use!

[davidorme]

Thanks @dalonsoa, really useful.

I have no problem with harsh - although I haven't always been rigorous about using these in my own (sole developer) projects, I think they are hugely valuable. I have a plan to let the team test all of this stuff on safedata_validator, which is win-win for getting that up to scratch and seeing how things work.

I don't mind about flake8 vs. pylint. pylint includes isorting (I don't know if it directly uses isort?) and I think is generally thought to be pickier, more annoying - probably good - and slower - which is bad. Either way, we can look at both - I think they are generally thought to be complementary rather than contradictory.

Using pre-commit to have a standard configuration for git pre-commit hooks, sounds great. Having the whole chain in pre-commit hooks sounds scary, but I agree makes sense.

The template looks great - I'm not as familiar with poetry and the shift from setup.py to pyproject.toml is new, but I can see the attraction of the venv to PyPi workflow!