GIScience
Introduce automated code style formatting and linting
The current state
Currently, this project does not have any default code style, neither is there any static code analysis in place. I am strongly convinced that introducing both would result in better, easier maintainable and easier-to-contribute-to code.
What I think we should do
I'd like to use black as a code formatter and flake8 and pylint for static code analysis.
What that would fix
$ flake8 | wc -l
461
$ pylint ORStools
…
Your code has been rated at 3.19/10
$ black ORStools
…
29 files reformatted.
Notes
Git-blame supports ignoring specific revisions - one big reformatting-commit could be ignored by any git-blame command run on the repo.
Need to check correct settings for all the exceptions like wrapper classes using camel case for method names and similar. Not that straight forward in this repo due to c++ dependencies.
This should also fix relative imports all around.
Additionally, note that .ui and …UI.py-files are automatically generated and should not be touched.
We have a few type annotations in some places. We should probably expand on this and use mypy for type checking at some point.
Regarding line length: This is a count of how many lines exceed the flake8 default line length of 79 characters.
$ flake8 --exclude __pycache__,*UI.py,*.ui,resources_rc.py | grep -o '[0-9]\+ >'|awk '{print $1}' | sort -h | uniq -c
6 80
12 81
16 82
8 83
11 84
7 85
8 86
6 87
8 88
10 89
8 90
9 91
5 92
10 93
4 94
9 95
4 96
3 97
9 98
2 99
6 100
3 101
3 102
6 103
2 104
3 105
5 106
2 107
6 108
3 109
2 110
5 111
3 112
5 113
4 114
4 115
5 116
3 117
3 118
1 126
In total, these are 229 out of roughly 8000 lines. Of these, 82 have line length <= 88 (black default line length).
On line length, there's a great piece of talk by Raymond Hettinger, python core dev. He recommends 90-ish as line length - meaning to not worry too much if it inches over the edge by a few characters, but rather to take it as a hint that maybe something is going wrong.
I'd therefore opt to look into the ~70 Lines w/ a line length over 100 and maybe into the other 50 w/ line length between 88 and 100 :)
We have a few type annotations in some places. We should probably expand on this and use mypy for type checking at some point.
Did you know this is built in in PyCharm? (Hint)
Main point for character limit is to be able to have files open next to each other and still be able to read the code properly. With resolution of our monitors in addition to multi monitor setup, i would maybe opt a bit higher than the original 70.
I would rather opt for a hard limit than a 90-ish rule, as i can set the character limit within the IDE and it will automatically break at the limit and notify if lines are over the limit.
(still got it set to 120 which is the default taken over from intellij)
We could set this up as proposed in the black documentation here, using github actions.
The same Lint workflow could also include pylint, flake8 and possibly isort. Compare the Django workflow for this.
Local installations would have to run them manually (or wait for any PRs to go through linting) - or we use any form of git hooks, possibly via pre-commit.