Catch Flake8 error codes with multiple letters #52750

samestep · 2021-02-24T16:50:13Z

The Flake8 job has been passing on master despite giving warnings for over a month. This is because it has been using a regex that doesn't recognize error codes starting with multiple letters, such as those used by flake8-executable. This PR corrects the regex, and also adds another step at the end of the job which asserts that Flake8 actually gave no error output, in case similar regex issues appear in the future.

Tagging the following people to ask what to do to fix these EXE002 warnings:

[distributed_test_c10d]: Enable disabled ROCm tests. #50629 authored by @jaglinux, approved by @rohan-varma
- test/distributed/test_c10d.py
DataPipe naming convension update #51262 authored by @glaringlee, approved by @ejguan
- torch/utils/data/datapipes/__init__.py
- torch/utils/data/datapipes/iter/loadfilesfromdisk.py
- torch/utils/data/datapipes/iter/listdirfiles.py
- torch/utils/data/datapipes/iter/__init__.py
- torch/utils/data/datapipes/utils/__init__.py
- torch/utils/data/datapipes/utils/common.py
move Tar Dataset to Tar DataPipe #51398 authored by @glaringlee, approved by @ejguan
- torch/utils/data/datapipes/iter/readfilesfromtar.py
move ZipDataset to Zip DataPipe #51599 authored by @glaringlee, approved by @ejguan
- torch/utils/data/datapipes/iter/readfilesfromzip.py
move RoutedDecoder Dataset to DataPipe #51704 authored by @glaringlee, approved by @ejguan
- torch/utils/data/datapipes/iter/routeddecoder.py
- torch/utils/data/datapipes/utils/decoder.py
move GroupByFilename Dataset to DataPipe #51709 authored by @glaringlee, approved by @ejguan
- torch/utils/data/datapipes/iter/groupbykey.py

Specifically, the question is: for each of those files, should we remove the execute permissions, or should we add a shebang? And if the latter, which shebang?

Test plan:

The Lint / flake8-py3 job in GitHub Actions:

this run failed, showing that the new regex catches these warnings properly
this run succeeded and gave no output in the "Run flake8" step, showing that this PR fixed all Flake8 warnings
this run (in [do not merge] Catch all Flake8 warnings #52755) failed, showing that the new last step of the job successfully catches Flake8 warnings even without the regex fix

facebook-github-bot · 2021-02-24T16:50:21Z

💊 CI failures summary and remediations

As of commit cf99a77 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

janeyx99

good catch!

samestep · 2021-02-24T16:52:31Z

good catch!

@janeyx99 lol don't approve yet, the Flake8 failures are still present :P

janeyx99 · 2021-02-24T16:55:37Z

@janeyx99 lol don't approve yet, the Flake8 failures are still present :P

Can still approve cuz your code can be landed, after the fixes happen independently. :p

facebook-github-bot

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

walterddr

looks good but cant we fix all those EXE002 in the same PR. those are just permission issues right. getting rid of it by chmod -x <file> should be good enough?

samestep · 2021-02-24T16:58:31Z

looks good but cant we fix all those EXE002 in the same PR. those are just permission issues right. getting rid of it by chmod -x <file> should be good enough?

@walterddr sure, I can go ahead and do that now

facebook-github-bot

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

walterddr · 2021-02-24T17:06:51Z

thanks. i dont think any of these datapipe utils are meant to be used as executable since there's no even __main__ defined. for test_c10d.py it seems should be invoked via run_tests (and of which is executable)

ejguan · 2021-02-24T17:15:57Z

Thanks for catching the error. But, do you have any suggestion to solve it for DataPipes? They should not be considered as executable which means shebang is not required.
I am trying to figure it out and will let you know if I find anything.

samestep · 2021-02-24T17:17:30Z

Thanks for catching the error. But, do you have any suggestion to solve it for DataPipes? They should not be considered as executable which means shebang is not required.
I am trying to figure it out and will let you know if I find anything.

@ejguan Currently this PR just removes those executable permissions, so we should be good to go. Thanks for confirming!

facebook-github-bot

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-02-24T20:58:39Z

@samestep merged this pull request in 5e977d9.

Summary: The Flake8 job has been passing on `master` despite giving warnings for [over a month](https://github.com/pytorch/pytorch/runs/1716124347). This is because it has been using a regex that doesn't recognize error codes starting with multiple letters, such as those used by [flake8-executable](https://pypi.org/project/flake8-executable/). This PR corrects the regex, and also adds another step at the end of the job which asserts that Flake8 actually gave no error output, in case similar regex issues appear in the future. Tagging the following people to ask what to do to fix these `EXE002` warnings: - pytorch#50629 authored by jaglinux, approved by rohan-varma - `test/distributed/test_c10d.py` - pytorch#51262 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/__init__.py` - `torch/utils/data/datapipes/iter/loadfilesfromdisk.py` - `torch/utils/data/datapipes/iter/listdirfiles.py` - `torch/utils/data/datapipes/iter/__init__.py` - `torch/utils/data/datapipes/utils/__init__.py` - `torch/utils/data/datapipes/utils/common.py` - pytorch#51398 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/readfilesfromtar.py` - pytorch#51599 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/readfilesfromzip.py` - pytorch#51704 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/routeddecoder.py` - `torch/utils/data/datapipes/utils/decoder.py` - pytorch#51709 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/groupbykey.py` Specifically, the question is: for each of those files, should we remove the execute permissions, or should we add a shebang? And if the latter, which shebang? Pull Request resolved: pytorch#52750 Test Plan: The **Lint / flake8-py3** job in GitHub Actions: - [this run](https://github.com/pytorch/pytorch/runs/1972039886) failed, showing that the new regex catches these warnings properly - [this run](https://github.com/pytorch/pytorch/runs/1972393293) succeeded and gave no output in the "Run flake8" step, showing that this PR fixed all Flake8 warnings - [this run](https://github.com/pytorch/pytorch/pull/52755/checks?check_run_id=1972414849) (in pytorch#52755) failed, showing that the new last step of the job successfully catches Flake8 warnings even without the regex fix Reviewed By: walterddr, janeyx99 Differential Revision: D26637307 Pulled By: samestep fbshipit-source-id: 572af6a3bbe57f5e9bd47f19f37c39db90f7b804

Summary: The Flake8 job has been passing on `master` despite giving warnings for [over a month](https://github.com/pytorch/pytorch/runs/1716124347). This is because it has been using a regex that doesn't recognize error codes starting with multiple letters, such as those used by [flake8-executable](https://pypi.org/project/flake8-executable/). This PR corrects the regex, and also adds another step at the end of the job which asserts that Flake8 actually gave no error output, in case similar regex issues appear in the future. Tagging the following people to ask what to do to fix these `EXE002` warnings: - #50629 authored by jaglinux, approved by rohan-varma - `test/distributed/test_c10d.py` - #51262 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/__init__.py` - `torch/utils/data/datapipes/iter/loadfilesfromdisk.py` - `torch/utils/data/datapipes/iter/listdirfiles.py` - `torch/utils/data/datapipes/iter/__init__.py` - `torch/utils/data/datapipes/utils/__init__.py` - `torch/utils/data/datapipes/utils/common.py` - #51398 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/readfilesfromtar.py` - #51599 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/readfilesfromzip.py` - #51704 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/routeddecoder.py` - `torch/utils/data/datapipes/utils/decoder.py` - #51709 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/groupbykey.py` Specifically, the question is: for each of those files, should we remove the execute permissions, or should we add a shebang? And if the latter, which shebang? Pull Request resolved: #52750 Test Plan: The **Lint / flake8-py3** job in GitHub Actions: - [this run](https://github.com/pytorch/pytorch/runs/1972039886) failed, showing that the new regex catches these warnings properly - [this run](https://github.com/pytorch/pytorch/runs/1972393293) succeeded and gave no output in the "Run flake8" step, showing that this PR fixed all Flake8 warnings - [this run](https://github.com/pytorch/pytorch/pull/52755/checks?check_run_id=1972414849) (in #52755) failed, showing that the new last step of the job successfully catches Flake8 warnings even without the regex fix Reviewed By: walterddr, janeyx99 Differential Revision: D26637307 Pulled By: samestep fbshipit-source-id: 572af6a3bbe57f5e9bd47f19f37c39db90f7b804 Co-authored-by: Sam Estep <sestep@fb.com>

Summary: The Flake8 job has been passing on `master` despite giving warnings for [over a month](https://github.com/pytorch/pytorch/runs/1716124347). This is because it has been using a regex that doesn't recognize error codes starting with multiple letters, such as those used by [flake8-executable](https://pypi.org/project/flake8-executable/). This PR corrects the regex, and also adds another step at the end of the job which asserts that Flake8 actually gave no error output, in case similar regex issues appear in the future. Tagging the following people to ask what to do to fix these `EXE002` warnings: - pytorch#50629 authored by jaglinux, approved by rohan-varma - `test/distributed/test_c10d.py` - pytorch#51262 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/__init__.py` - `torch/utils/data/datapipes/iter/loadfilesfromdisk.py` - `torch/utils/data/datapipes/iter/listdirfiles.py` - `torch/utils/data/datapipes/iter/__init__.py` - `torch/utils/data/datapipes/utils/__init__.py` - `torch/utils/data/datapipes/utils/common.py` - pytorch#51398 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/readfilesfromtar.py` - pytorch#51599 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/readfilesfromzip.py` - pytorch#51704 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/routeddecoder.py` - `torch/utils/data/datapipes/utils/decoder.py` - pytorch#51709 authored by glaringlee, approved by ejguan - `torch/utils/data/datapipes/iter/groupbykey.py` Specifically, the question is: for each of those files, should we remove the execute permissions, or should we add a shebang? And if the latter, which shebang? Pull Request resolved: pytorch#52750 Test Plan: The **Lint / flake8-py3** job in GitHub Actions: - [this run](https://github.com/pytorch/pytorch/runs/1972039886) failed, showing that the new regex catches these warnings properly - [this run](https://github.com/pytorch/pytorch/runs/1972393293) succeeded and gave no output in the "Run flake8" step, showing that this PR fixed all Flake8 warnings - [this run](https://github.com/pytorch/pytorch/pull/52755/checks?check_run_id=1972414849) (in pytorch#52755) failed, showing that the new last step of the job successfully catches Flake8 warnings even without the regex fix Reviewed By: walterddr, janeyx99 Differential Revision: D26637307 Pulled By: samestep fbshipit-source-id: 572af6a3bbe57f5e9bd47f19f37c39db90f7b804

Catch Flake8 error codes with multiple letters

bce6e2e

samestep requested review from rohan-varma, glaringlee, ejguan and a team February 24, 2021 16:50

facebook-github-bot added the cla signed label Feb 24, 2021

janeyx99 approved these changes Feb 24, 2021

View reviewed changes

facebook-github-bot reviewed Feb 24, 2021

View reviewed changes

walterddr reviewed Feb 24, 2021

View reviewed changes

Remove executable permissions from the files

f10ac5d

samestep requested review from mrshenli, pritamdamania87 and zhaojuanmao as code owners February 24, 2021 17:05

facebook-github-bot reviewed Feb 24, 2021

View reviewed changes

Add a last step to catch future unmatched warnings

cf99a77

samestep mentioned this pull request Feb 24, 2021

[do not merge] Catch all Flake8 warnings #52755

Closed

facebook-github-bot reviewed Feb 24, 2021

View reviewed changes

walterddr approved these changes Feb 24, 2021

View reviewed changes

malfet approved these changes Feb 24, 2021

View reviewed changes

facebook-github-bot closed this in 5e977d9 Feb 24, 2021

facebook-github-bot added the Merged label Feb 24, 2021

This was referenced Feb 25, 2021

[v1.8] Catch Flake8 error codes with multiple letters #52801

Merged

[v.1.8.0] Release Tracker #51886

Closed

This was referenced Mar 8, 2021

Bump torch from 1.7.1 to 1.8.0 Rose-STL-Lab/torchTS#7

Closed

Bump torch from 1.4.0 to 1.8.0 JadHADDAD92/covid-mask-detector#23

Closed

dependabot bot mentioned this pull request Mar 16, 2021

[tune](deps): Bump torch from 1.7.1 to 1.8.0 in /python/requirements michaelzhiluo/ray#12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Catch Flake8 error codes with multiple letters #52750

Catch Flake8 error codes with multiple letters #52750

samestep commented Feb 24, 2021 •

edited

facebook-github-bot commented Feb 24, 2021 •

edited

janeyx99 left a comment

samestep commented Feb 24, 2021

janeyx99 commented Feb 24, 2021 •

edited

facebook-github-bot left a comment

walterddr left a comment

samestep commented Feb 24, 2021 •

edited

facebook-github-bot left a comment

walterddr commented Feb 24, 2021

ejguan commented Feb 24, 2021

samestep commented Feb 24, 2021

facebook-github-bot left a comment

facebook-github-bot commented Feb 24, 2021

Catch Flake8 error codes with multiple letters #52750

Catch Flake8 error codes with multiple letters #52750

Conversation

samestep commented Feb 24, 2021 • edited

facebook-github-bot commented Feb 24, 2021 • edited

💊 CI failures summary and remediations

janeyx99 left a comment

Choose a reason for hiding this comment

samestep commented Feb 24, 2021

janeyx99 commented Feb 24, 2021 • edited

facebook-github-bot left a comment

Choose a reason for hiding this comment

walterddr left a comment

Choose a reason for hiding this comment

samestep commented Feb 24, 2021 • edited

facebook-github-bot left a comment

Choose a reason for hiding this comment

walterddr commented Feb 24, 2021

ejguan commented Feb 24, 2021

samestep commented Feb 24, 2021

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Feb 24, 2021

samestep commented Feb 24, 2021 •

edited

facebook-github-bot commented Feb 24, 2021 •

edited

janeyx99 commented Feb 24, 2021 •

edited

samestep commented Feb 24, 2021 •

edited