What to do about flake8 #1927
aucampia
started this conversation in
Development
What to do about flake8
#1927
Replies: 7 comments 10 replies
-
However, it requires a large amount of human resources. |
Beta Was this translation helpful? Give feedback.
0 replies
-
|
Problem can be significantly reduced with relatively trivial, albeit time-consuming, effort ... ignoring line-length complaints and some irrelevant problem it has with line 0: I count 46 PRs 😄 rdflib/_type_checking.py
rdflib/collection.py
rdflib/compare.py
rdflib/compat.py
rdflib/extras/external_graph_libs.py
rdflib/extras/infixowl.py
rdflib/graph.py
rdflib/namespace/__init__.py
rdflib/parser.py
rdflib/plugins/parsers/hext.py
rdflib/plugins/parsers/jsonld.py
rdflib/plugins/parsers/notation3.py
rdflib/plugins/parsers/trig.py
rdflib/plugins/parsers/trix.py
rdflib/plugins/serializers/jsonld.py
rdflib/plugins/serializers/longturtle.py
rdflib/plugins/serializers/nt.py
rdflib/plugins/serializers/rdfxml.py
rdflib/plugins/serializers/turtle.py
rdflib/plugins/serializers/xmlwriter.py
rdflib/plugins/shared/jsonld/context.py
rdflib/plugins/shared/jsonld/util.py
rdflib/plugins/sparql/__init__.py
rdflib/plugins/sparql/algebra.py
rdflib/plugins/sparql/datatypes.py
rdflib/plugins/sparql/evaluate.py
rdflib/plugins/sparql/evalutils.py
rdflib/plugins/sparql/operators.py
rdflib/plugins/sparql/parser.py
rdflib/plugins/sparql/parserutils.py
rdflib/plugins/sparql/results/jsonresults.py
rdflib/plugins/sparql/results/tsvresults.py
rdflib/plugins/sparql/sparql.py
rdflib/plugins/sparql/update.py
rdflib/plugins/stores/concurrent.py
rdflib/plugins/stores/memory.py
rdflib/plugins/stores/sparqlconnector.py
rdflib/plugins/stores/sparqlstore.py
rdflib/query.py
rdflib/term.py
rdflib/tools/csv2rdf.py
rdflib/tools/defined_namespace_creator.py
rdflib/tools/rdf2dot.py
rdflib/tools/rdfs2dot.py
rdflib/util.py |
Beta Was this translation helpful? Give feedback.
4 replies
-
|
The main concern here is that it can't be done very gradually, and if you also install pep8-naming the problem becomes even more severe, and this excludes tests which I would also prefer be pep8 compliant, at least for new tests. |
Beta Was this translation helpful? Give feedback.
-
Beta Was this translation helpful? Give feedback.
-
|
@eggplants thanks for raising this |
Beta Was this translation helpful? Give feedback.
-
|
peterjc/flake8-black#59 (comment) |
Beta Was this translation helpful? Give feedback.
-
|
I think |
Beta Was this translation helpful? Give feedback.
3 replies
-
sed -i -e '335s/except:/except Exception:/' rdflib/query.py
sed -i -e '113s/except:/except Exception:/' rdflib/compat.py
sed -i -e '102s/except:/except Exception:/' rdflib/tools/rdf2dot.py
sed -i -e '117s/except:/except Exception:/' rdflib/tools/rdf2dot.py
sed -i -e '420s/except:/except Exception:/' rdflib/tools/csv2rdf.py
sed -i -e '90s/except:/except Exception:/' rdflib/tools/rdfs2dot.py
sed -i -e '108s/except:/except Exception:/' rdflib/plugins/parsers/trix.py
sed -i -e '125s/except:/except Exception:/' rdflib/plugins/parsers/trix.py
sed -i -e '332s/except:/except Exception:/' rdflib/plugins/parsers/notation3.py
sed -i -e '1650s/except:/except Exception:/' rdflib/plugins/parsers/notation3.py
sed -i -e '256s/except:/except Exception:/' rdflib/plugins/serializers/rdfxml.py
sed -i -e '129s/except:/except Exception:/' rdflib/plugins/serializers/longturtle.py
sed -i -e '250s/except:/except Exception:/' rdflib/plugins/serializers/longturtle.py
sed -i -e '282s/except:/except Exception:/' rdflib/plugins/serializers/turtle.py
sed -i -e '407s/except:/except Exception:/' rdflib/plugins/serializers/turtle.py |
Beta Was this translation helpful? Give feedback.
-
|
flake8-bugbear's error message is better... 38 B001 Do not use bare |
Beta Was this translation helpful? Give feedback.
-
Thanks for the heads-up on bugbear, it had some useful suggestions to make for infixowl and actually revealed a case in which none of the loop variables were being used. |
Beta Was this translation helpful? Give feedback.
-
|
In black environment, |
Beta Was this translation helpful? Give feedback.
0 replies
-
|
I would like to create a PR for each error for those that could be easily corrected. (When I have time...) |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
I think it would be great, though I do think I will go with flakeheaven for now, hopefully we can disable it soon but it will be very helpful for new PRs. |
Beta Was this translation helpful? Give feedback.
-
$ flake8 rdflib|awk '$0=$2'|sort|uniq -c|sort -rnk1
89 BLK999 # https://github.com/peterjc/flake8-black/issues/59
42 E203 # suppress `whitespace before ':'`
37 E402 # move the import statements to the top if no prob
24 E722 # replace `except:` with `except Exception:`
13 F401 # delete unused var or import statements if no prob
4 E741 # replace ambiguous 'l' with 'line'
3 F841 # delete unused local variable if no prob
3 F821 # ?
3 F811 # ?
2 F402 # ?
1 F403 # ?
1 E266 # replace '## ...comment...' with '# ..comment...' |
Beta Was this translation helpful? Give feedback.
2 replies
-
|
I'm thinking to remove flake8-black, it seems to not be helpful, and it is not really needed. |
Beta Was this translation helpful? Give feedback.
-
|
I agree with you. should be removed for now. |
Beta Was this translation helpful? Give feedback.
-
|
Made a PR here to add flakeheaven: |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Currently we run flake8 but essentially ignore all errors, this is not very helpful, and there definitely are useful things we want from flake8 (and plugins), including things like warning about unused imports, ambigious variable names, bad indentation, unused variables, bare except usage.
There are some options I see to improve the situation:
I'm somewhat leaning towards 3, and almost done with a PR for it, but I would like to know what others think.
I think with all options flake8 should be reported independently so it does not get mixed in with mypy and testing failures, that way reviewers can also decide if these failures are worth worrying about, I also think there may be some warnings we eventually decide to disable entirely.
The reason this is relevant is because we recently have many new PRs that are non compliant with PEP8 in variant ways, one way being that they tend to include function names that are camel case, and I would prefer a computer raises these concerns for us as that reduces the amount of work we have to do in reviews.
CC: @RDFLib/core @eggplants @cclauss
Beta Was this translation helpful? Give feedback.
All reactions