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

Add DefaultDict to typing.py #179

Closed
gvanrossum opened this issue Jan 29, 2016 · 9 comments · Fixed by #197
Closed

Add DefaultDict to typing.py #179

gvanrossum opened this issue Jan 29, 2016 · 9 comments · Fixed by #197

Comments

@gvanrossum
Copy link
Member

I've seen some PY2 code that uses defaultdict[t1, t2] in type annotations. Those are inside comments so it works. But if you try to do the same using inline PY3 annotations it doesn't work:

from collections import defaultdict
def foo() -> defaultdict[str, int]:
    d = defaultdict(int)
    return d

This gives an error:

TypeError: 'type' object is not subscriptable

We should probably just add typing.DefaultDict (similar to the way we added NamedTuple).

@ilevkivskyi
Copy link
Member

If people are already using this, and the solution is simple, then this is a good idea. I added a PR #197 for this

@gvanrossum
Copy link
Member Author

I like this, but we should also add this to mypy. Can you submit a PR there too?

@ilevkivskyi
Copy link
Member

I am not sure about this. I am not familiar with all the internals of mypy. Do I need to simply make changes to lib-typing/3.2 and lib-typing/2.7 or something else also? If only lib-typing should be modified, then no problem.

@gvanrossum
Copy link
Member Author

gvanrossum commented Apr 3, 2016 via email

@gvanrossum
Copy link
Member Author

gvanrossum commented Apr 3, 2016 via email

@ilevkivskyi
Copy link
Member

@gvanrossum I am now working on adding this to mypy. I tried to add typing.DefaultDict as an alias for collections.defaultdict to nodes.py and DefaultDict = TypeAlias(object) to typing.pyi (in exactly the same way as it is done for Dict and dict), but this didn't work. On the other hand, adding a single line DefaultDict = collections.defaultdict to typing.pyi stub works perfectly.

While I was adding tests for this I found out that python/mypy#815 has not been fixed completely. Here are few examples:

from collections import defaultdict

# this is now fixed
defaultdict(list)

# this expectedly fails but the error message shows that the type of first argument 
# incorrectly found to be List[_T], while it should be type, or something similar:
# Argument 1 to "defaultdict" has incompatible type List[_T]; expected Callable[[], str]
d = defaultdict(list) # type: defaultdict[int, str]

class MyDDict(defaultdict):
    pass

# and here we see the old bug again
# Argument 1 to "MyDDict" has incompatible type List[_T]; expected Callable[[], _VT]
MyDDict(list)

@gvanrossum
Copy link
Member Author

Hm... If you just alias typing.DefaultDict to collections.defaultdict, you won't be able to index it in a position where the code is executable, e.g. in a Python 3 style annotation def foo(x: DefaultDict[str, int]) nor as a base class, e.g. class DD(DefaultDict[str, int]). Mypy will accept these because the stub declares collections.defaultdict as generic, but the real stdlib doesn't. That's why we need variants that are generic at run time in typing.py. (How you do it in typing.pyi can be different.)

Can you please separately file the bug with MDDict(list)? It appears mypy may not be handling an overloaded __init__ found in a base class correctly.

@gvanrossum
Copy link
Member Author

(Sorry, I missed some context. I blame the lack of coffee. :-) I think just adding typing.DefaultDict as an alias for collections.defaultdict in typeshed is the way to go, since collections.defaultdict is fully generic (and we can't change this easily because there's already too much code depending on it).

@ilevkivskyi
Copy link
Member

@gvanrossum , OK I filed python/mypy#1358 and added a PR to typeshed python/typeshed#139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants