Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OrderedDict fails disallow_any_generics, but OrderedDict[Any, Any] doesn't run #7539

Closed
l0b0 opened this issue Sep 19, 2019 · 9 comments · Fixed by #7707
Closed

OrderedDict fails disallow_any_generics, but OrderedDict[Any, Any] doesn't run #7539

l0b0 opened this issue Sep 19, 2019 · 9 comments · Fixed by #7707

Comments

@l0b0
Copy link

l0b0 commented Sep 19, 2019

I've been trying to introduce disallow_any_generics = true to an already extensively typed code base of ~48kSLOC, and the only remaining issue is typing OrderedDicts.

Actual behaviour

Given this configuration:

[mypy]
check_untyped_defs = true
disallow_any_generics = true
disallow_untyped_defs = true
ignore_missing_imports = true
no_implicit_optional = true
warn_redundant_casts = true
warn_return_any = true
warn_unused_ignores = true

Code with implicit Any:

from collections import OrderedDict
from typing import Any

def name(value: OrderedDict) -> None:
    pass

This fails validation because of disallow_any_generics = true:

$ mypy example.py
example.py:4: error: Missing type parameters for generic type

Code with typed OrderedDict:

from collections import OrderedDict
from typing import Any

def name(value: OrderedDict[Any, Any]) -> None:
    pass

This passes validation, but doesn't run:

$ python example.py 
Traceback (most recent call last):
  File "example.py", line 4, in <module>
    def name(value: OrderedDict[Any, Any]) -> None:
TypeError: 'type' object is not subscriptable

Versions:

$ python --version
Python 3.6.8
$ mypy --version
mypy 0.720

(Also failed on mypy 0.711.)

Expected behaviour

I'd expect either the first snippet to be valid or the second snippet to run. As it is I'm stuck for how to proceed.

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 20, 2019

You can use string literal escapes, such as value: 'OrderedDict[Any, Any]'. Another option is from __future__ import annotations (introduced in Python 3.7).

@dms-cat
Copy link

dms-cat commented Sep 20, 2019

@JukkaL Is there no way to do this without quoting the type? Because even if I if TYPE_CHECKING: from collections import OrderedDict that triggers an issue in flake8:

F401 'collections.OrderedDict' imported but unused

@JelleZijlstra
Copy link
Member

You can use from __future__ import annotations in 3.7+.

@asottile
Copy link
Contributor

do you want typing.OrderedDict and not collections.OrderedDict for that annotation?

@JelleZijlstra
Copy link
Member

I was going to say that too but it turns out there is no typing.OrderedDict.

@asottile
Copy link
Contributor

there is in 3.7 iirc

@JelleZijlstra
Copy link
Member

Oh, you're right, it is in 3.7 (https://github.com/python/cpython/blob/3.7/Lib/typing.py) but not in the typing repo (https://github.com/python/typing/blob/master/src/typing.py) where I checked first. I filed python/typing#679 to add it.

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 23, 2019

If mypy sees that OrderedDict (or another class that doesn't support indexing at runtime) is missing required generic type args, we could give a better error message that suggests using string literal escaping.

@ilevkivskyi
Copy link
Member

Relevant docs https://mypy.readthedocs.io/en/latest/common_issues.html#using-classes-that-are-generic-in-stubs-but-not-at-runtime

ilevkivskyi pushed a commit that referenced this issue Oct 21, 2019
This commit introduces a recommendation for classes that don't support indexing at runtime to be escaped as string literals on missing required generic type args (BARE_GENERIC) error.

It should resolve #7539
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants