frozenleaves/pytorch
1
0
Fork 0
mirror of https://github.com/pytorch/pytorch.git synced 2025-10-20 21:14:14 +08:00
Code Issues Packages Projects Releases Wiki Activity

Add lint for unqualified noqa (#56272)

Browse Source 
Summary:
As this diff shows, currently there are a couple hundred instances of raw `noqa` in the codebase, which just ignore all errors on a given line. That isn't great, so this PR changes all existing instances of that antipattern to qualify the `noqa` with respect to a specific error code, and adds a lint to prevent more of this from happening in the future.

Interestingly, some of the examples the `noqa` lint catches are genuine attempts to qualify the `noqa` with a specific error code, such as these two:
```
test/jit/test_misc.py:27:            print(f"{hello + ' ' + test}, I'm a {test}") # noqa E999
test/jit/test_misc.py:28:            print(f"format blank") # noqa F541
```
However, those are still wrong because they are [missing a colon](https://flake8.pycqa.org/en/3.9.1/user/violations.html#in-line-ignoring-errors), which actually causes the error code to be completely ignored:

- If you change them to anything else, the warnings will still be suppressed.
- If you add the necessary colons then it is revealed that `E261` was also being suppressed, unintentionally:
  ```
  test/jit/test_misc.py:27:57: E261 at least two spaces before inline comment
  test/jit/test_misc.py:28:35: E261 at least two spaces before inline comment
  ```

I did try using [flake8-noqa](https://pypi.org/project/flake8-noqa/) instead of a custom `git grep` lint, but it didn't seem to work. This PR is definitely missing some of the functionality that flake8-noqa is supposed to provide, though, so if someone can figure out how to use it, we should do that instead.

Pull Request resolved: https://github.com/pytorch/pytorch/pull/56272

Test Plan:
CI should pass on the tip of this PR, and we know that the lint works because the following CI run (before this PR was finished) failed:

- https://github.com/pytorch/pytorch/runs/2365189927

Reviewed By: janeyx99

Differential Revision: D27830127

Pulled By: samestep

fbshipit-source-id: d6dcf4f945ebd18cd76c46a07f3b408296864fcb
This commit is contained in:
Sam Estep
2021-04-19 13:14:27 -07:00
committed by Facebook GitHub Bot
parent 7bcf95bbb6
commit e3900d2ba5
75 changed files with 259 additions and 275 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
test/test_dispatch.py
View File

@ -264,7 +264,7 @@ Inferred operator schema for a C++ kernel function doesn't match the expected fu
registered at /dev/null:0
inferred schema: (Tensor _0) -> (Tensor _0)
impl_t_t
reason: The number of arguments is different. 2 vs 1.''') # noqa
reason: The number of arguments is different. 2 vs 1.''')
def test_def_with_inference(self):
state = self.commute("foo", [
@ -722,7 +722,7 @@ alias analysis kind: PURE_FUNCTION
self.commute("foo", ops, expect_raises=True).state,
'''Tried to register an operator (test::foo(Tensor x) -> (Tensor)) with the same name and overload '''
'''name multiple times. Each overload's schema should only be registered with a single call to def(). '''
'''Duplicate registration: registered at /dev/null:0. Original registration: registered at /dev/null:0''' # noqa
'''Duplicate registration: registered at /dev/null:0. Original registration: registered at /dev/null:0'''
)
def test_multiple_fallback(self):
@ -734,7 +734,7 @@ alias analysis kind: PURE_FUNCTION
self.assertExpectedInline(
str(e),
'''Tried to register multiple backend fallbacks for the same dispatch key XLA; previous registration '''
'''registered at /dev/null:0, new registration registered at /dev/null:0''' # noqa
'''registered at /dev/null:0, new registration registered at /dev/null:0'''
)
else:
self.assertTrue(False)