Pep8 compliance

[espg]

What is pep8?

Pep8 is the official style guide for python, it defines conventions to make code cleaner and easier to understand. There is automated checking for pep8 compliance using flake8.

How is pgamit doing with flake8 right now?

Poorly. We have 3,719 pep8 violations in our codebase-- i.e., starting here and going to here.

Here's the breakdown by violation code/type:

76    C901 'insert_modify_param' is too complex (29)
4     E101 indentation contains mixed spaces and tabs
1     E111 indentation is not a multiple of 4
3     E114 indentation is not a multiple of 4 (comment)
29    E115 expected an indented block (comment)
7     E116 unexpected indentation (comment)
1     E117 over-indented
29    E122 continuation line missing indentation or outdented
3     E124 closing bracket does not match visual indentation
1     E125 continuation line with same indent as next logical line
29    E127 continuation line over-indented for visual indent
52    E128 continuation line under-indented for visual indent
3     E129 visually indented line with same indent as next logical line
7     E131 continuation line unaligned for hanging indent
24    E201 whitespace after '{'
16    E202 whitespace before '}'
310   E203 whitespace before ':'
2     E211 whitespace before '('
1007  E221 multiple spaces before operator
8     E222 multiple spaces after operator
18    E225 missing whitespace around operator
188   E231 missing whitespace after ','
699   E251 unexpected spaces around keyword / parameter equals
27    E261 at least two spaces before inline comment
19    E262 inline comment should start with '# '
78    E265 block comment should start with '# '
3     E266 too many leading '#' for block comment
2     E271 multiple spaces after keyword
43    E272 multiple spaces before keyword
4     E301 expected 1 blank line, found 0
20    E302 expected 2 blank lines, found 1
32    E303 too many blank lines (2)
1     E305 expected 2 blank lines after class or function definition, found 1
3     E401 multiple imports on one line
5     E402 module level import not at top of file
50    E501 line too long (133 > 127 characters)
50    E502 the backslash is redundant between brackets
27    E701 multiple statements on one line (colon)
7     E703 statement ends with a semicolon
20    E711 comparison to None should be 'if cond is None:'
4     E713 test for membership should be 'not in'
2     E714 test for object identity should be 'is not'
74    E722 do not use bare 'except'
5     E731 do not assign a lambda expression, use a def
9     E741 ambiguous variable name 'l'
81    F401 'pgamit.pyETM.DEFAULT_FREQUENCIES' imported but unused
1     F402 import 'StationInstance' from line 16 shadowed by loop variable
1     F403 'from pgamit import *' used; unable to detect undefined names
35    F405 'pyRinexName' may be undefined, or defined from star imports: pgamit
4     F541 f-string is missing placeholders
3     F811 redefinition of unused 'pyProducts' from line 41
34    F841 local variable 'config' is assigned to but never used
4     W191 indentation contains tabs
161   W291 trailing whitespace
2     W292 no newline at end of file
237   W293 blank line contains whitespace
22    W391 blank line at end of file
132   W605 invalid escape sequence '\s'
3719

Most of these are very minor and easy to fix, but there are enough of them that it will take time.

Do we really need to fix these? It seems like a lot of work...

These are 'warnings' rather than 'errors', so we don't have to fix them-- but we shouldn't add more, and can fix them slowly during the general software development process.

So what do you recommend?

I'm working on an 'on pull_request' github action that will run for PR's and fail to pass checks if the modified code isn't pep8 compliant. This will ensure that the new code we produce doesn't make the situation worse moving forward. I'll also add instructions to (and create) our contributors guide for this.

As @pdsmith90 , @demiangomez , myself, and others get used to checking for flake8 on code, we'll also be working on enhancements, bugfixes, and updates to existing code files in pgamit. Because we'll get flagged for pep8 on those files during the PR process, we can start to fix the backlog above in 15 and 20 minute chunks as we interact with the existing codebase, which should spread out the effort and make it less noticeable in terms of effort spent on this.