PEP8: bulk fixes on entire grass7/ subdirectory tree with flake8

[neteler]

While not being a big fan of bulk fixes I have taken liberty to apply flake8 recursively on grass-addons/grass7/.

The fixes are separated by commit in order to more easily review them.
Eventually, after review, all can be squashed into one commit.

Rationale: follow more closely our coding style: https://trac.osgeo.org/grass/wiki/Submitting/Python#PEP8standardStyle

Once accepted, the GitHub Action flake8 test from core GRASS GIS may be used also here.

The script I wrote to auto-create this PR:

cat pep8_formatting_commit.sh 
#!/bin/sh

# requires "python3-autopep8"

# put desired error codes to be fixed into CSV file
# https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes
#
# format:
# E117 over-indented
# E122 continuation line missing indentation or outdented
# ...

CODES=~/pep8_formatting_codes_addons.csv

# trick: using a non-english char as a special separator
for i in $(cat $CODES | sed 's+ +ä+g') ; do
    code="$(echo $i | cut -d'ä' -f1)"
    desc=$(echo $i | cut -d'ä' -f2- | sed 's+ä+ +g')
    echo "Processing: $code: $desc..."
    ag --python -l | xargs autopep8 --select $code --aggressive --aggressive --in-place --jobs 4
    LISTTOCOMMIT=$(git status | grep modified | tr -s ' ' ' ' | cut -d' ' -f2)
    git commit -m"PEP8: fix $code: $desc" $LISTTOCOMMIT
done

[neteler]

To simplify the review, below the non-trivial changes:

(use mouse-over to see the description)

• 65ce2b6
• 23b9baf
• 755cf54
• fa2b282
• 0c4f642
• 9abbd58
• bf103a5

Leftover python debugger?

• 7e5fde8

The other commits in this PR are wrong blanks, semicolons, mixed tab/white space, and such being cleaned up.

[wenzeslaus]

Thanks, I checked what you linked. In the following, a manual change would be better:

# grass7/gui/wxpython/wx.rdigit/rdigit/dialogs_core.py
-   UsingGDAL  = vExternalOut.has_key('Not using GDAL') == False
+   UsingGDAL  = "Not using GDAL" not in vExternalOut

Although that looks like super fragile code in any case.

You are right about import ipdb; ipdb.set_trace(). It should be just deleted.

If I remember correctly, bulk autopep run on wxGUI code broke several places mainly due to bad replacement of tabs by spaces (perhaps autopep did not expect tab to be used for character number reduction). It would be good to check files which had tabs on lines with some code (the relevant commit has many files with tabs only at empty lines).

[neteler]

Thanks @wenzeslaus - changed as suggested.

[hellik]

scrolled through my addons and a few others where I've been involved (e.g. r.basin, r.hazard.flood, r.hypso, ...), looks good so far

[hellik]

from the submitting guidelines:

    Do not use space around parentheses: 

# use this:
grass.run_command('g.region', raster='myrast')
# not this:
grass.run_command( 'g.region', raster='myrast' )

though in the bulk refactoring only the first space around the first parentheses is deleted, the second is still there.

for example:

grass.message( "Starting importing GBIF data ..." ) 

changed in

grass.message("Starting importing GBIF data ..." )

[neteler]

though in the bulk refactoring only the first space around the first parentheses is deleted, the second is still there.

Indeed. While this rule here https://www.flake8rules.com/rules/E201.html states

"Open parentheses should not have any space before or after them."

(which was in this PR done with 1205fa6)

I do not find the corresponding "Closing parentheses ..." pendant for it.

Does anyone know the flake8 code to be applied here?

[neteler]

... found it :-)

E202: whitespace before ‘)’

[wenzeslaus]

1. The whitespace would be ultimately best handled by Black I think, but this will get it closer at least for single line statements.
2. The E202 issue surprised me. I thought you are actually running flake8 on it afterwards, but it does not seem so.
3. Any idea about list of files which had a tab at a non-whitespace line? These would be the ones to check manually (see above).

[neteler]

1. The whitespace would be ultimately best handled by Black I think, but this will get it closer at least for single line statements.

I suggest to separate Black application out into a new PR. This PR is already large enough.

2. The E202 issue surprised me. I thought you are actually running flake8 on it afterwards, but it does not seem so.

Not sure what you mean. I simply overlooked to apply E202 initially when preparing this PR.

3. Any idea about list of files which had a tab at a non-whitespace line? These would be the ones to check manually (see above).

I checked and identified these related commits:
(use mouse-over to see the description)

• 2ac5f0f
• 46e7451
• 3e7ea28
• 1bfa3b5
• 876bec7

I did a shallow test of wx.metadata related commands and it worked ok.

[wenzeslaus]

1. The whitespace would be ultimately best handled by Black I think, but this will get it closer at least for single line statements.

I suggest to separate Black application out into a new PR. This PR is already large enough.

I agree. Just saying the Black whitespace is different than the Flake8 (still PEP8/Flake8 complaint, but more strict and sometimes different than other auto-formatting tools.

2. The E202 issue surprised me. I thought you are actually running flake8 on it afterwards, but it does not seem so.

Not sure what you mean. I simply overlooked to apply E202 initially when preparing this PR.

I mean running the Flake8 checks. It seemed to me you are talking about auto-applying changes.

3. Any idea about list of files which had a tab at a non-whitespace line? These would be the ones to check manually (see above).

I checked and identified these related commits:

• 2ac5f0f
• 46e7451
• 3e7ea28
• 1bfa3b5
• 876bec7

These look good. No changes in if branches.

[neteler]

Is there anything else left open or may I merge?