[Bug] Fix currently ignored Flake8 warnings

[wenzeslaus]

Fix one or more of the issues reported by Flake8.

This issue is not something which will be fixed by one PR. Instead, it serves as a pointer to the list of ignored warnings in the source code. Pick one or more issues or files, fix them, and submit a PR.

Getting the existing issues to fix

1. Go to the source code.
2. Find one of the .flake8 files in the source tree.
3. See the per-file-ignores section.
4. Pick a file, group of files, issues, or group of issues, e.g., E741 or utils/mkrest.py.
5. Delete that from the .flake8 file.
6. Run flake8 --show-source --statistics in the directory where the edited .flake8 file is.
7. See where the errors are.

Note that all issues in the per-file-ignores section should be fixed unless noted otherwise. Some Flake8 warnings may be false positives or perhaps there is a good reason for the code to break some of the best practices. In that case, the issue should be ignored at the particular line using # noqa: ... with a comment stating the reason for it.

Finding .flake8 files

Currently, there is several .flake8 files. To find them use, e.g., find:

find . -name ".flake8"

or ls

ls .flake8 */.flake8 */*/.flake8

The current list is:

./.flake8
./gui/wxpython/.flake8
./lib/init/.flake8
./python/grass/.flake8
./scripts/.flake8
./temporal/.flake8

Example

At the time of writing, the file ./scripts/.flake8 contains line i.in.spotvgt/i.in.spotvgt.py: E722 in the per-file-ignores section. Searching online for "Flake8 E722" leads to Flake8 Rules: Do not use bare except, specify exception instead page which explains the issue and a general guide on how to fix it.

[ShubhamSwati]

Hi, I'm new to open source environments and coding, but I would like to work on this issue. If you can guide me a bit...

[wenzeslaus]

Hi, that's great. Get the basic stuff first, like source code through Git and compile the source code (easiest on Linux, e.g., on Ubuntu). Then you can start making modifications.

Given that GRASS GIS is not only Python, you need to recompile to test the code. What to test and how might be difficult to guess first, but often it won't be even needed depending on what you decide to fix from these issues, so I would just worry about it once you have an open PR to keep things simple at the beginning.

I'm adding this info here for convenience, but the place to discuss this further is grass-dev mailing list. You don't need to subscribe, but it is the best place to discuss technical issues with your setup, compilation etc. You can also create an issue to ask for a better documentation if you can't find some information in the existing resources.

[nilason]

@echoix This issue might be a challenge to you :-) The list is long...

[echoix]

I didn't see that issue yet, but I started a bit on my own without knowing. The part that I said "Should I make another PR with the missing fixes", it's because I had them in another branch (the commits that followed what I sent as a PR)

[arohanajit]

As per the this entry on .flake8:
man/build_html.py: E501
The line too long error seems to be occuring in the html snippets of the code. Since this error is not an error in the code and would mess up html format in that particular snippet - is it required to be fixed?
alternatively increasing max-line-length should also fix this

[echoix]

As per the this entry on .flake8:

man/build_html.py: E501

The line too long error seems to be occuring in the html snippets of the code. Since this error is not an error in the code and would mess up html format in that particular snippet - is it required to be fixed?

Can you point to the snippets you'd like an answer on? Probably they could be split without problems, html doesn't have to be exactly character perfect to still work and do the same thing

[arohanajit]

Here are a couple of examples:
86: r"""<!-- the files grass${grass_version_major}.html & helptext.html file live in lib/init/ -->
94: <li class="box"><span>Index of <a href="topics.html">topics</a> and <a href="keywords.html">keywords</a></span></li>

These contain very little "texts" and more of tags and parameters, yet both of them trigger the warning. This reocurring pattern is basically leading to multiple E501

[wenzeslaus]

Note: There is #3748 open which addresses E722 (do not use bare except), so that needs to be merged first before working specifically on E722.

[wenzeslaus]

...well, or rewritten in a new PR or abandoned. There is a ton of conflicts. Have you already analyzed that @echoix by any chance?

[echoix]

...well, or rewritten in a new PR or abandoned. There is a ton of conflicts. Have you already analyzed that @echoix by any chance?

I'm note sure I understand what you are talking about, nor with the comment above.

But regarding flake8, no, I didn't play with it really. I really prefer to work with ruff for rules that auto fix, or just for the rules they have implemented as the feedback cycle is so fast it just runs as you type in an IDE. So as a side effect, it helps for all the other linters, but partially (on what is implemented).

So probably some ruff fixes I did helped some of flake8. But things like too many variables, too many functions, aren't really automatically actionnable. Remember that we started with a little debt, and we have a project with something like 391000 lines of Python code, that represents 31% of the lines of code of the project (last time I counted this summer).

[wenzeslaus]

That's fine. Great answer. Sorry for not being clear. I will try to get to #3748 and see if how that compares to the Ruff changes.