flake8 for codestyle

[itrharrison]

I have added a workflow which tests code against the PEP8 specification for python code style as per one of the todos in #29 .

I made one adjustment, which is to allow lines up to 100 characters long, rather than just 70.

You can test your codestyle locally by running
tox -e codestyle
in the repo directory.

I have fixed up all of the existing code to get it to a state where it passes. This was mostly trivial, but there were a few places where things were flagged as unused or undefined which I guess are underdevelopment. Rather than delete these things I have commented them out, and will raise Issues for bug fixes where appropriate.

Some common gotchas:

• spaces around operators (+, -, = etc)
• spaces after commas, colons
• trailing whitespace and whitespace on blank lines

Checking for codestyle like this can seem a bit annoying, but it is also a very good way to find bugs as well as keeping your code tidy.

[timothydmorton]

Have you explored at all using pre-commit checks? That can enforce styles like this on commit so people don't have to worry about manually fixing style changes when merging.

[cmbant]

might be worth making the style consistent with cobaya which checks E713,E704,E703,E714,E741,E10,E11,E20,E22,E23,E25,E27,E301,E302,E304,E9,F405,F406,F5,F6,F7,F8,W1,W2,W3,W6 and uses line length 90?

Precommit hooks sound like a good idea, but can they be enforced without each user installing the check for their local git?

[timothydmorton]

👍 for making consistent with cobaya. I think the definitions of what are in the checks can be made part of the repo, and then each dev user would need to run pre-commit install for themselves.

[itrharrison]

Thanks both, I am still learning my way with this kind of thing.

I will make the style checks consistent with cobaya, which seems sensible given the expected integration.

On pre-commit vs part of the Actions... I would maybe lean towards keeping it as part of the Actions, as it feels more of a proper barrier for people, as otherwise they could just not run pre-commit, then the burden of code style and related bugs goes onto the reviewer (I think this is how it would work?).

If we did go for pre-commit, presumably pre-commit could also be added to the requirements?

[itrharrison]

A third way could be having both. Encouraging people to pre-commit but leaving in the Action.

[cmbant]

Should have it in the action anyway (presumably would just always pass if pre-commit check done). Hopefully most people will use PyCharm or similar, in which case they'd use the built-in checking, not sure there's a way to make integrated/consistent with pre-commit. If in doubt I would leave it to the user..

[itrharrison]

Okay, I will merge this PR as it is, including the new Action, and I will also add a note suggesting people run the pre-commit checks as part of #29 .

As far as I can tell both the pre-commit and Actions runs of flake8 look at setup.cfg the same, so things should stay consistent (I will check this though).