Add `warn_unused_ignores = True` to our mypy config #8231

pradyunsg · 2020-05-13T11:47:18Z

We should add warn_unused_ignores = True in our mypy global configuration in a follow up PR.

Originally posted by @pradyunsg in #7932 (comment)

This would catch "unused" / redundant # type: ignore comments, and allow us to remove them when they're no longer necessary.

The text was updated successfully, but these errors were encountered:

deveshks · 2020-05-13T13:15:33Z

I added warn_unused_ignores = True in setup.cfg's mypy section and on running tox -e lint, the following warnings were generated.

mypy errors

mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

src/pip/_internal/models/direct_url.py:41: error:
unused
'type:
ignore'
comment
            expected_type = six.string_types ...
            ^
src/pip/_internal/utils/compat.py:46: error:
unused
'type:
ignore'
comment
            cache_from_source = imp.cache_fro...
            ^
src/pip/_internal/utils/unpacking.py:186: error:
unused
'type:
ignore'
comment
                    fn = split_leading_dir(fn...
                    ^
src/pip/_internal/operations/install/wheel.py:68: error:
unused
'type:
ignore'
comment
        return (digest, str(length))  # type:...
        ^
src/pip/_internal/req/req_file.py:412: error:
unused
'type:
ignore'
comment
                shlex.split(options_str), def...
                ^
src/pip/_internal/req/req_file.py:434: error:
unused
'type:
ignore'
comment
        return ' '.join(args), ' '.join(optio...
        ^
src/pip/_internal/cli/main_parser.py:74: error:
unused
'type:
ignore'
comment
            sys.stdout.write(parser.version) ...
            ^
src/pip/_internal/commands/debug.py:41: error:
unused
'type:
ignore'
comment
            implementation = sys.implementati...
            ^
Found 8 errors in 7 files (checked 135 source files)

mypy, for Python 2.......................................................Failed
- hook id: mypy
- exit code: 1

src/pip/_internal/req/req_file.py:408: error:
unused
'type:
ignore'
comment
                options_str = options_str.enc...
                ^
Found 1 error in 1 file (checked 132 source files)

So in the PR for this, do we also remove the unused #type: ignore comments to get rid of these warnings? If yes, I will go ahead and create a PR adding the warn_unused_ignores option and remove the unnecessary type: ignore: (I guess we would also need to fix other mypy errors which come up after removing them)

deveshks · 2020-05-13T13:48:26Z

After getting rid of the type: ignore from the lines for which we were warned in the above comment, we got new errors from mypy as follows. Most of them were from mypy 2, only one is from mypy 3.

mypy errors are removing unnecessary type: ignore

mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

src/pip/_internal/req/req_file.py:408: error:
Incompatible
types
in
assignment
(expression
has
type
"bytes",
variable
has
type
"str")
    ...      options_str = options_str.encode('u...
                           ^
Found 1 error in 1 file (checked 135 source files)

mypy, for Python 2.......................................................Failed
- hook id: mypy
- exit code: 1

src/pip/_internal/utils/compat.py:46: error:
Module
has
no
attribute
"cache_from_source"
    ...cache_from_source = imp.cache_from_source
                           ^
src/pip/_internal/models/direct_url.py:41: error:
Incompatible
types
in
assignment
(expression
has
type
"Tuple[Type[str], Type[unicode]]",
variable
has
type
"Type[T]")
            expected_type = six.string_types
                            ^
src/pip/_internal/req/req_file.py:412: error:
Argument
1
to
"split"
has
incompatible
type
"unicode";
expected
"Optional[str]"
    ...        shlex.split(options_str), default...
                           ^
src/pip/_internal/req/req_file.py:434: error:
Incompatible
return
value
type
(got
"Tuple[unicode, unicode]",
expected
"Tuple[str, unicode]")
        return ' '.join(args), ' '.join(optio...
               ^
src/pip/_internal/utils/unpacking.py:186: error:
Incompatible
types
in
assignment
(expression
has
type
"Union[str, unicode]",
variable
has
type
"str")
    ...               fn = split_leading_dir(fn)...
                           ^
src/pip/_internal/operations/install/wheel.py:68: error:
Incompatible
return
value
type
(got
"Tuple[unicode, str]",
expected
"Tuple[str, str]")
        return (digest, str(length))
                ^
src/pip/_internal/cli/main_parser.py:74: error:
Argument
1
to
"write"
of
"IO"
has
incompatible
type
"unicode";
expected
"str"
            sys.stdout.write(parser.version)
                             ^
src/pip/_internal/commands/debug.py:41: error:
Module
has
no
attribute
"implementation"
    ...      implementation = sys.implementation
                              ^
Found 8 errors in 7 files (checked 132 source files)

pradyunsg · 2020-05-13T20:17:39Z

Gah, looks like I should remove the --pretty that was added to our mypy runs.

pradyunsg · 2020-05-13T20:24:08Z

Okay, looks like we gotta keep this until we kill Python 2 support. Until we remove that, we can't remove some of those comments. We're triggering 2 mypy runs, that hit 2 separate "bunches" of # type: ignore comments. :)

deveshks · 2020-05-13T20:27:36Z

Okay, looks like we gotta keep this until we kill Python 2 support. :)

Agreed, the effort we will put to correct the types after removing type: ignore just for Python 2 is not worth it, given we are going to remove Python 2 support after a few days.

deveshks · 2020-05-13T20:29:48Z

Gah, looks like I should remove the --pretty that was added to our mypy runs.

Do you want me to create a PR to do that. The difference in displaying mypy errors is pretty obvious

with --pretty

src/pip/_internal/req/req_file.py:408: error:
Incompatible
types
in
assignment
(expression
has
type
"bytes",
variable
has
type
"str")
    ...      options_str = options_str.encode('u...
                           ^
Found 1 error in 1 file (checked 135 source files)

without --pretty

src/pip/_internal/req/req_file.py:408: error: Incompatible types in assignment (expression has type "bytes", variable has type "str")
Found 1 error in 1 file (checked 135 source files)

pradyunsg · 2020-05-13T20:36:49Z

Do you want me to create a PR to do that. The difference in displaying mypy errors is pretty obvious

Yea, a PR removing --pretty + a bug report on mypy that --pretty doesn’t work with pre-commit on Windows, is the best thing to do here IMO.

deveshks · 2020-05-13T20:49:03Z

Yea, a PR removing --pretty

#8235 created to remove it

a bug report on mypy that --pretty doesn’t work with pre-commit on Windows

May I know more details on what the issue here exactly is? I myself have macOS and I didn't face any issue involving pre-commit and --pretty.

pradyunsg · 2020-05-13T21:43:41Z

src/pip/_internal/req/req_file.py:408: error:
Incompatible
types
in
assignment
(expression
has
type
"bytes",
variable
has
type
"str")
... options_str = options_str.encode('u...
^
Found 1 error in 1 file (checked 135 source files)

@deveshks You see this behavior right? Could you report that mypy is printing one-word-per-line when --pretty is passed, and used with pre-commit?

deveshks · 2020-05-13T22:02:47Z

Could you report that mypy is printing one-word-per-line when --pretty is passed, and used with pre-commit?

Done in python/mypy#8816 . Sorry the mention of Windows threw me off and I confused the issue with something else.

hexagonrecursion · 2021-02-10T07:05:17Z

It looks like you forgot to close this issue.

$ fgrep warn_unused_ignores setup.cfg 
warn_unused_ignores = True
$ tox -e lint
lint installed: appdirs==1.4.4,cfgv==3.2.0,distlib==0.3.1,filelock==3.0.12,identify==1.5.13,nodeenv==1.5.0,pip==21.0.1,pre-commit==2.10.1,PyYAML==5.4.1,setuptools==52.0.0,six==1.15.0,toml==0.10.2,virtualenv==20.4.2,wheel==0.36.2
lint run-test-pre: PYTHONHASHSEED='726417819'
lint run-test: commands[0] | pre-commit run --all-files --show-diff-on-failure --hook-stage=manual
Check builtin type constructor use.......................................Passed
Check for added large files..............................................Passed
Check for case conflicts.................................................Passed
Check Toml...............................................................Passed
Check Yaml...............................................................Passed
Debug Statements (Python)................................................Passed
Fix End of Files.........................................................Passed
Forbid new submodules....................................................Passed
Trim Trailing Whitespace.................................................Passed
black....................................................................Passed
flake8...................................................................Passed
isort....................................................................Passed
mypy.....................................................................Passed
use logger.warning(......................................................Passed
check for eval().........................................................Passed
rst ``code`` is two backticks............................................Passed
NEWS fragment........................................(no files to check)Skipped
check-manifest...........................................................Passed
___________________________________ summary ____________________________________
  lint: commands succeeded
  congratulations :)

uranusjr · 2021-02-10T12:10:38Z

Yup, the config was added in #9340. Thanks for catching this.

triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label May 13, 2020

pradyunsg added state: awaiting PR Feature discussed, PR is needed type: maintenance Related to Development and Maintenance Processes labels May 13, 2020

triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label May 13, 2020

pradyunsg added Python 2 only Python 2 specific state: blocked Can not be done until something else is done and removed state: awaiting PR Feature discussed, PR is needed labels May 13, 2020

deveshks mentioned this issue May 13, 2020

Update mypy to v0.770 #8235

Merged

uranusjr closed this as completed Feb 10, 2021

github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `warn_unused_ignores = True` to our mypy config #8231

Add `warn_unused_ignores = True` to our mypy config #8231

pradyunsg commented May 13, 2020 •

edited

Loading

deveshks commented May 13, 2020 •

edited

Loading

deveshks commented May 13, 2020 •

edited

Loading

pradyunsg commented May 13, 2020

pradyunsg commented May 13, 2020 •

edited

Loading

deveshks commented May 13, 2020

deveshks commented May 13, 2020

pradyunsg commented May 13, 2020

deveshks commented May 13, 2020

pradyunsg commented May 13, 2020

deveshks commented May 13, 2020

hexagonrecursion commented Feb 10, 2021

uranusjr commented Feb 10, 2021

Add warn_unused_ignores = True to our mypy config #8231

Add warn_unused_ignores = True to our mypy config #8231

Comments

pradyunsg commented May 13, 2020 • edited Loading

deveshks commented May 13, 2020 • edited Loading

deveshks commented May 13, 2020 • edited Loading

pradyunsg commented May 13, 2020

pradyunsg commented May 13, 2020 • edited Loading

deveshks commented May 13, 2020

deveshks commented May 13, 2020

pradyunsg commented May 13, 2020

deveshks commented May 13, 2020

pradyunsg commented May 13, 2020

deveshks commented May 13, 2020

hexagonrecursion commented Feb 10, 2021

uranusjr commented Feb 10, 2021

Add `warn_unused_ignores = True` to our mypy config #8231

Add `warn_unused_ignores = True` to our mypy config #8231

pradyunsg commented May 13, 2020 •

edited

Loading

deveshks commented May 13, 2020 •

edited

Loading

deveshks commented May 13, 2020 •

edited

Loading

pradyunsg commented May 13, 2020 •

edited

Loading