Skip to content

Remove unused type: ignore comments#56402

Closed
samestep wants to merge 2 commits intopytorch:masterfrom
samestep:mypy-warn-unused-ignores
Closed

Remove unused type: ignore comments#56402
samestep wants to merge 2 commits intopytorch:masterfrom
samestep:mypy-warn-unused-ignores

Conversation

@samestep
Copy link
Copy Markdown
Contributor

@samestep samestep commented Apr 19, 2021

A prerequisite for #56290. This PR enables mypy's warn_unused_ignores in our non-strict mypy.ini config, and removes all existing unused type: ignore comments.

Possible downsides of doing this:

  1. This could cause un-ignorable errors on files that are checked by both mypy.ini and mypy-strict.ini.
  2. This may cause mypy errors that appear on some platforms or environments but not on others.

Test plan:

CI, or run mypy locally.

@samestep samestep requested a review from a team April 19, 2021 19:42
@facebook-github-bot facebook-github-bot added cla signed oncall: jit Add this issue/PR to JIT oncall triage queue labels Apr 19, 2021
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Apr 19, 2021 •
edited
Loading

💊 CI failures summary and remediations

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

  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See GitHub Actions build mypy (1/1)

Step: "Run mypy" (full log | diagnosis details | 🔁 rerun)

2021-04-19T19:47:15.3169626Z tools/stats_utils/...mentation or library stub for module named 'boto3' 
2021-04-19T19:47:10.0903094Z ##[group]Run set -eux
2021-04-19T19:47:10.0903521Z �[36;1mset -eux�[0m
2021-04-19T19:47:10.0903931Z �[36;1mfor CONFIG in mypy*.ini; do mypy --config="$CONFIG"; done�[0m
2021-04-19T19:47:10.0937793Z shell: /bin/bash -e {0}
2021-04-19T19:47:10.0938086Z env:
2021-04-19T19:47:10.0938522Z   pythonLocation: /opt/hostedtoolcache/Python/3.8.9/x64
2021-04-19T19:47:10.0939117Z   LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.8.9/x64/lib
2021-04-19T19:47:10.0939568Z ##[endgroup]
2021-04-19T19:47:10.1005740Z + for CONFIG in mypy*.ini
2021-04-19T19:47:10.1007026Z + mypy --config=mypy-strict.ini
2021-04-19T19:47:15.3169626Z tools/stats_utils/s3_stat_parser.py:11:1: error: Cannot find implementation or library stub for module named 'boto3'
2021-04-19T19:47:15.3172245Z tools/stats_utils/s3_stat_parser.py:11:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
2021-04-19T19:47:28.0164599Z Found 1 error in 1 file (checked 56 source files)
2021-04-19T19:47:28.6975733Z ##[error]Process completed with exit code 1.
2021-04-19T19:47:28.7076163Z Post job cleanup.
2021-04-19T19:47:28.8122286Z [command]/usr/bin/git version
2021-04-19T19:47:28.8178672Z git version 2.31.1
2021-04-19T19:47:28.8219587Z [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
2021-04-19T19:47:28.8253521Z [command]/usr/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :
2021-04-19T19:47:28.8484672Z [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
2021-04-19T19:47:28.8515779Z http.https://github.com/.extraheader
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.

@facebook-github-bot facebook-github-bot added oncall: distributed Add this issue/PR to distributed oncall triage queue fx labels Apr 19, 2021
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Apr 19, 2021

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

@samestep
Copy link
Copy Markdown
Contributor Author

samestep commented Apr 19, 2021

Closing this because I think the downsides actually do outweigh the benefits (at least, the benefits beyond what #56290 would provide).

@samestep samestep closed this Apr 19, 2021
@samestep samestep deleted the mypy-warn-unused-ignores branch April 19, 2021 19:57
@samestep samestep mentioned this pull request May 28, 2021
Closed
facebook-github-bot pushed a commit that referenced this pull request Jun 7, 2021
@samestep @facebook-github-bot
Strictly type everything in .github and tools (#59117)
737d920 
Summary:
This PR greatly simplifies `mypy-strict.ini` by strictly typing everything in `.github` and `tools`, rather than picking and choosing only specific files in those two dirs. It also removes `warn_unused_ignores` from `mypy-strict.ini`, for reasons described in #56402 (comment): basically, that setting makes life more difficult depending on what libraries you have installed locally vs in CI (e.g. `ruamel`).

Pull Request resolved: #59117

Test Plan:
```
flake8
mypy --config mypy-strict.ini
```

Reviewed By: malfet

Differential Revision: D28765386

Pulled By: samestep

fbshipit-source-id: 3e744e301c7a464f8a2a2428fcdbad534e231f2e
deniskokarev pushed a commit to deniskokarev/pytorch that referenced this pull request Jun 9, 2021
@samestep @deniskokarev
Strictly type everything in .github and tools (pytorch#59117)
cb37145 
Summary:
This PR greatly simplifies `mypy-strict.ini` by strictly typing everything in `.github` and `tools`, rather than picking and choosing only specific files in those two dirs. It also removes `warn_unused_ignores` from `mypy-strict.ini`, for reasons described in pytorch#56402 (comment): basically, that setting makes life more difficult depending on what libraries you have installed locally vs in CI (e.g. `ruamel`).

Pull Request resolved: pytorch#59117

Test Plan:
```
flake8
mypy --config mypy-strict.ini
```

Reviewed By: malfet

Differential Revision: D28765386

Pulled By: samestep

fbshipit-source-id: 3e744e301c7a464f8a2a2428fcdbad534e231f2e
@samestep samestep mentioned this pull request Jun 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albanD albanD Awaiting requested review from albanD

@BowenBao BowenBao Awaiting requested review from BowenBao

@H-Huang H-Huang Awaiting requested review from H-Huang

@jbschlosser jbschlosser Awaiting requested review from jbschlosser

@mingzhe09088 mingzhe09088 Awaiting requested review from mingzhe09088

@mrshenli mrshenli Awaiting requested review from mrshenli

@neginraoof neginraoof Awaiting requested review from neginraoof

@pritamdamania87 pritamdamania87 Awaiting requested review from pritamdamania87

@rohan-varma rohan-varma Awaiting requested review from rohan-varma

@wayi1 wayi1 Awaiting requested review from wayi1

@soulitzer soulitzer Awaiting requested review from soulitzer

@spandantiwari spandantiwari Awaiting requested review from spandantiwari

@zhaojuanmao zhaojuanmao Awaiting requested review from zhaojuanmao

Assignees

No one assigned

Labels

cla signed oncall: distributed Add this issue/PR to distributed oncall triage queue oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@samestep @facebook-github-bot