Flake8 Template Change to Reduce Warnings #21

rnickle · 2021-11-13T02:45:21Z

Thank you for this excellent tool, which I am using on several projects.

When I run genbadge for flake8, I notice two things:

I am using flake8-comments 0.1.2, flake8-type-annotations 0.1.0, and flake8-broken-line 0.4.0 (the latter two by the same author.) All three output the flake8 code with a colon, which does not match genbadge's flake8 parser and yields a warning. This patch modifies the regex to allow statistics line with or without that extra colon and quiets the warnings.

$ egrep -e '^[0-9]+\s+[A-Z0-9]+[\s:]\s' reports/flake8/flake8stats.txt~manywarnings 
3     CM001: Redundant comment found
4     N400: Found backslash that is used for line breaking
2     T800: Missing spaces between parameter annotation and default value

Also, when I write the output of flake8 --statistics to --output-file, many warnings from genbadge are generated for flake8 output ("Line in Flake8 statistics report does not match template") for the normal lines in the report of the filename, the source line, the column, the code and the message. Although it might be possible to suppress these, I would prefer to keep them in the output and suppress the warning. This patch parses lines that do not match genbadge's flake8 template, extracts the filename, and if the filename corresponds to a source file it will suppress the warning as a way to verify that it was a source-related output from flake8 rather than some other type of unexpected output. Example of a line that triggers the warning.

directory/sourcefile.py:139:33: Q000 Single quotes found but double quotes preferred

Thank you for considering this pull request, and please let me know if I can improve the submission.

Also ignore lines that are source file reports.

Fewer Warnings for Flake 8

smarie · 2021-11-17T14:24:48Z

genbadge/utils_flake8.py

@@ -104,7 +105,8 @@ def get_flake8_stats(flake8_stats_file):
    return parse_flake8_stats(flake8_stats_txt)


-RE_TO_MATCH = re.compile(r"([0-9]+)\s+([A-Z0-9]+)\s.*")
+RE_TO_MATCH = re.compile(r"([0-9]+)\s+([A-Z0-9]+):*\s.*")


Suggested change

RE_TO_MATCH = re.compile(r"([0-9]+)\s+([A-Z0-9]+):*\s.*")

# Note: the optional colon `:*` is to comply with some flake8 plugins, see PR #21

RE_TO_MATCH = re.compile(r"([0-9]+)\s+([A-Z0-9]+):*\s.*")

Thank you for reviewing this @smarie. I don't see why the very small patch related to the colon shouldn't be merged.

To be certain that this different format was the exception rather than some new format, I just did a quick review of the three plugins I found that have the colon:

flake8-comments 0.1.2 Was last updated 5 months ago, no comments or PRs.
flake8-type-annotations 0.4.0 Is active with a few comments and PRs.

To my surprise:

flake8-type-annotations 0.1.0 Is actually DEPRECATED, because the functionality was merged into flake8 under different rules, so I will stop using this one.

I don't see why the very small patch related to the colon shouldn't be merged.

It should ! The suggestion I made is about adding a comment above the line. For some reason github does not highlight the diff correctly.
Thanks for investigating this list of colon-creating plugins

smarie · 2021-11-17T14:28:06Z

Thanks a lot @rnickle for this submission (and for the kind words) ! I made a few minor suggestions.

It would be great if you could add a test with an example such flake8 --output file. That would illustrate the lines and format you're referring to.

I am wondering if the lines matching the source format should be ignored or should actually be parsed too. Indeed, the example you show above directory/sourcefile.py:139:33: Q000 Single quotes found but double quotes preferred is an actual flake8 issue, so it should be caught ; no ?

smarie · 2021-11-17T14:29:14Z

genbadge/utils_flake8.py

@@ -115,7 +117,14 @@ def parse_flake8_stats(stats_txt  # type: str
    for line in stats_txt.splitlines():
        match = RE_TO_MATCH.match(line)
        if not match:
-            warn("Line in Flake8 statistics report does not match template and will be ignored: %r" % line)
+            smatch = SOURCE_MATCH.match(line)


Suggested change

smatch = SOURCE_MATCH.match(line)

# Other supported lines formats ?

smatch = SOURCE_MATCH.match(line)

Yes, thank you @smarie, I did not realize that it wasn't being caught because I did not audit and verify the counts.

I have quite a few warnings from several flake8 plugins. I can organize the ones you don't already have into tests/reports/flake8/flake8stats.txt and cross-check the counts. I will report back on my findings, and if I introduce any changes to the code I will provide tests.

It seems to me that I should also provide tests for the ":" cases, so I will try testing against that also. I will try to turn this around over the next few days.

That would be great @rnickle ! My guess is that with one or two additional test files it should cover most of the variants, but I let you find investigate. Let me know !

smarie · 2021-11-17T14:30:22Z

genbadge/utils_flake8.py

@@ -115,7 +117,14 @@ def parse_flake8_stats(stats_txt  # type: str
    for line in stats_txt.splitlines():
        match = RE_TO_MATCH.match(line)
        if not match:


Now that there are several cases, it would probably be preferable to first handle the "match" (reverting the if). That would put the nominal case upfront.

Each case could start with a comment illustrating the kind of pattern.
For the nominal one for example # Nominal case, such as: "N400: Found backslash that is used for line breaking"

Pardon me for the delay, I looked into it a little, and just had a chance to go back. I was concerned about a variety of things, including exactly how find_severity worked. I added my flake8stats.txt and modified the tests to make sure they could pass with it.

smarie · 2021-11-17T14:32:20Z

genbadge/utils_flake8.py

-            warn("Line in Flake8 statistics report does not match template and will be ignored: %r" % line)
+            smatch = SOURCE_MATCH.match(line)
+            if smatch:
+                source = smatch.group(1)


Suggested change

source = smatch.group(1)

# Line with source file, such as "directory/sourcefile.py:139:33: Q000 Single quotes found but double quotes preferred"

source = smatch.group(1)

Adding a very large selection of flake8 results from a project that had never been refactored. The following flake8 plugins were enabled: flake8-aaa flake8-blind-except flake8-broken-line flake8-builtins flake8-class-attributes-order flake8-commas flake8-comments flake8-comprehensions flake8-docstrings flake8-dunder-all flake8-expression-complexity flake8-html flake8-if-expr flake8-import-order flake8-logging-format flake8-module-imports flake8-module-name flake8-pep3101 flake8-plugin-utils flake8-polyfill flake8-quotes flake8-raise flake8-rst-docstrings flake8-secure-coding-standard flake8-simplify flake8-string-format flake8-tuple flake8-type-annotations flake8-typehinting flake8-typing-imports pep8-naming

rnickle and others added 4 commits November 12, 2021 19:59

Adapt stats collection to allow for colon

9350f9e

Also ignore lines that are source file reports.

Add extra field for column.

0762b68

Make sure group returns str not tuple

4cdda0c

Merge pull request #1 from rnickle/fewerwarnings

a86ca4a

Fewer Warnings for Flake 8

smarie reviewed Nov 17, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flake8 Template Change to Reduce Warnings #21

Flake8 Template Change to Reduce Warnings #21

rnickle commented Nov 13, 2021

smarie Nov 17, 2021

rnickle Nov 18, 2021

smarie Nov 18, 2021

smarie commented Nov 17, 2021 •

edited

Loading

smarie Nov 17, 2021 •

edited

Loading

rnickle Nov 18, 2021

smarie Nov 18, 2021

smarie Nov 17, 2021 •

edited

Loading

rnickle Dec 12, 2021

smarie Nov 17, 2021

	RE_TO_MATCH = re.compile(r"([0-9]+)\s+([A-Z0-9]+):\s.")
	# Note: the optional colon `:*` is to comply with some flake8 plugins, see PR #21
	RE_TO_MATCH = re.compile(r"([0-9]+)\s+([A-Z0-9]+):\s.")

	smatch = SOURCE_MATCH.match(line)
	# Other supported lines formats ?
	smatch = SOURCE_MATCH.match(line)

	source = smatch.group(1)
	# Line with source file, such as "directory/sourcefile.py:139:33: Q000 Single quotes found but double quotes preferred"
	source = smatch.group(1)

Flake8 Template Change to Reduce Warnings #21

Are you sure you want to change the base?

Flake8 Template Change to Reduce Warnings #21

Conversation

rnickle commented Nov 13, 2021

smarie Nov 17, 2021

Choose a reason for hiding this comment

rnickle Nov 18, 2021

Choose a reason for hiding this comment

smarie Nov 18, 2021

Choose a reason for hiding this comment

smarie commented Nov 17, 2021 • edited Loading

smarie Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

rnickle Nov 18, 2021

Choose a reason for hiding this comment

smarie Nov 18, 2021

Choose a reason for hiding this comment

smarie Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

rnickle Dec 12, 2021

Choose a reason for hiding this comment

smarie Nov 17, 2021

Choose a reason for hiding this comment

smarie commented Nov 17, 2021 •

edited

Loading

smarie Nov 17, 2021 •

edited

Loading

smarie Nov 17, 2021 •

edited

Loading