Update code styling and linting

[CasperWA]

This is very much a developer's issue.

I'd like to suggest to update the Python code styling to use black, and otherwise use pylint for linting (instead of flake8 as the CI job is doing now).
Concerning the linting I don't have as strong feelings as I do with respect to the styling/formatting, however, in my experience pylint seems more transparent to me with better checks.

Furthermore, I'd like to add other code-checking tools to this code, to make sure it's secure and developer-friendly.
Taken directly from @quuat 's FastAPI template, I'd want to implement the following tools (in addition to black and pylint):

• bandit
  Checks code security, e.g., when using URL calls, etc.
• safety
  Checks all dependencies for known security vulnerabilities.
• mypy
  A static type checker.

The latter (mypy) might be "controversial", however, I think having a good typing allows for clearer code, since it's clear what a function will expect of inputs and what it will eventually return. Typing up the code also makes for better IDE experiences.

Personally, I would implement this as a combination between pre-commit and CI (GitHub Action job).
The pre-commit tool is quite neat as it can auto-reformat the code according to our chosen code styling before we commit, as well as sanity check our new code before committing.
However, it would need to be installed in every working clone one has by doing pre-commit install, although only once.
Also, it can always (but should never) be circumvented by passing the --no-verify option to git commit.
These reasons are why it should be implemented also in the CI.

The reason for not just having it in the CI is that the obvious mistakes get caught before committing and running external resources. This is very good and makes for quicker development, when we don't have to wait for stupid mistakes or typos to first become apparent after the CI job has finished.

As a note, I recently implemented these tools in a project of mine.

[CasperWA]

Should I go ahead with this? It will take a bit of work to create the PR, so I don't want to start it unless it's something we want done.

[francescalb]

Re: pylint - I have never used it so do not have strong opinions. As far as I have understood it is powerful but requires quite a bit of work on setting up rules for the project and can be a bit too 'rigid'. Is this correct?

Anyhow, I don't really have any opinions on this. I go with what you choose as I don't have experience to prefer one or the other,

[francescalb]

Re: bandit and safety

I have a vague memory that I added at least bandit once previously, but it appears I did not complete it. I am for both.

[francescalb]

pre-commit seems fine but is it much different than running flake8 and pytest locally before pushing?

[CasperWA]

pre-commit seems fine but is it much different than running flake8 and pytest locally before pushing?

No, it's exactly the automisation of this :)
Also, you don't need to install flake8 locally, it will be installed in a pre-commit-handled virtual environment.
For pylint this is different, since pylint also checks importability, so to work properly you shouldn't use pre-commit's virtual environments, but rather use the pylint that is installed locally (together with the package itself). In short, one still needs to install pylint locally whether using pre-commit or not.

For pytest, this is different. Pytest is a unit testing framework, not a styling or linting tool, and so it doesn't makes sense to add to pre-commit in my opinion - although it could be done. But running pytest every time you commit is cumbersome.

[francescalb]

mypy: sure, I guess this require quite some work on the code?

[CasperWA]

mypy: sure, I guess this require quite some work on the code?

Indeed. This is the most time-consuming to introduce (also pylint might become a bit time-consuming, but at least that is a clear advantage to satisfy, whereas for mypy the immediate advantages are a bit more... abstract).

[CasperWA]

Concerning mypy, I just found a reason why it's not so abstract anymore: It will greatly enhance the new API Reference documentation if we add static types to all functions, classes and methods. Hence, it would be good to add mypy at the same time to ensure the static typing is actually correct :)

[francescalb]

Concerning mypy, I just found a reason why it's not so abstract anymore: It will greatly enhance the new API Reference documentation if we add static types to all functions, classes and methods. Hence, it would be good to add mypy at the same time to ensure the static typing is actually correct :)

I see, even more for me to learn :)