Merge flake8 configurations #1172
Conversation
.flake8
Outdated
| ignore = E305,E402,E721,E741,F401,F403,F405,F999,W503,W504 | ||
| exclude = build,docs/source,_ext,third_party | ||
| ignore = E305,E402,E721,E741,F401,F403,F405,W503,W504,F821,F841,F999 | ||
| exclude = build,docs/source,_ext,third_party,docs/src,venv,torch/lib/gloo,torch/lib/pybind11,torch/lib/nanopb |
There was a problem hiding this comment.
Let's remove unused ones. Only build,docs/source,third_party are relevant and the others are originated from PyTorch core.
.flake8
Outdated
| max-line-length = 120 | ||
| ignore = E305,E402,E721,E741,F401,F403,F405,F999,W503,W504 | ||
| exclude = build,docs/source,_ext,third_party | ||
| ignore = E305,E402,E721,E741,F401,F403,F405,W503,W504,F821,F841,F999 |
There was a problem hiding this comment.
What happens if F821 and F841 are not added here? They should not be added here I think.
There was a problem hiding this comment.
Can you also see what happens if removing 401, 402, 403, 405, 503, 504? I do not see a good reason in torchaudio that they should be ignored.
There was a problem hiding this comment.
There was a problem hiding this comment.
I agree that we should remove F821, F841
There was a problem hiding this comment.
F503, F504 looks like its being ignore in pytorch too.
https://www.flake8rules.com/rules/W504.html
https://www.flake8rules.com/rules/W503.html
From what I understand they seem to be contradicting each other
There was a problem hiding this comment.
403,405 being ignore by pytorch too. (Rule discouraging the use of * imports)
https://www.flake8rules.com/rules/F403.html
https://www.flake8rules.com/rules/F405.html
There was a problem hiding this comment.
looks like we have 401 violations.
There was a problem hiding this comment.
Would it possible for you to fix it?
For __init__.pys, listing the imported stuff in __all__ would do. https://stackoverflow.com/a/31079085
For others, unused import should be removed.
If that uncovers other issues, (like unused imported function is used from external module). We can just give up and do it in a follow up.
|
@krishnakalyan3 I am merging this one now. If you would like to follow up on the change to the rule. Feel free to open a new PR. |
|
Thanks! |
3 participants
#1053