Support type inference for defaultdict() #8167

JukkaL · 2019-12-18T14:42:26Z

This allows inferring type of x, for example:

from collections import defaultdict

x = defaultdict(list)  # Infer defaultdict[str, List[int]]
x['foo'].append(1)

The implemention is not pretty and we have probably
reached about the maximum reasonable level of special
casing in type inference now.

There is a hack to work around the problem with leaking
type variable types in nested generics calls (I think).
This will break some (likely very rare) use cases.

This allows inferring type of `x`, for example: ``` from collections import defaultdict x = defaultdict(list) # defaultdict[str, List[int]] x['foo'].append(1) ``` The implemention is not pretty and we have probably reached about the maximum reasonable level of special casing in type inference now. There is a hack to work around the problem with leaking type variable types in nested generics calls (I think). This will break some (likely very rare) use cases.

ilevkivskyi

Thanks! This is the last big missing piece for type inference of empty collections. I have some comments, also why do you need the typeshed change?

ilevkivskyi · 2019-12-18T15:05:33Z

mypy/checker.py

+                arg1 = get_proper_type(init_type.args[1])
+                if (isinstance(arg0, (NoneType, UninhabitedType)) and
+                        isinstance(arg1, Instance) and
+                        self.is_valid_ordereddict_partial_value_type(arg1)):


This name is really weird:

Why ordereddict and not defaultdict?

I don't think it should be called valid, maybe empty?

For example is_empty_defaultdict_value_type().

Oops, I'll rename it.

ilevkivskyi · 2019-12-18T15:06:31Z

mypy/checker.py

+                arg0 = get_proper_type(init_type.args[0])
+                arg1 = get_proper_type(init_type.args[1])
+                if (isinstance(arg0, (NoneType, UninhabitedType)) and
+                        isinstance(arg1, Instance) and


It would be cleaner to move this check to the below helper.

Yeah. It's here since we rely on the narrowed down type below, so we'd need a type check anyway.

ilevkivskyi · 2019-12-18T15:07:04Z

mypy/checker.py

@@ -2813,14 +2813,24 @@ def infer_partial_type(self, name: Var, lvalue: Lvalue, init_type: Type) -> bool
            partial_type = PartialType(None, name)
        elif isinstance(init_type, Instance):
            fullname = init_type.type.fullname
-            if (isinstance(lvalue, (NameExpr, MemberExpr)) and
+            is_ref = isinstance(lvalue, (NameExpr, MemberExpr))


While you are at it maybe change this to RefExpr?

Makes sense.

ilevkivskyi · 2019-12-18T15:07:51Z

mypy/checker.py

@@ -2829,6 +2839,19 @@ def infer_partial_type(self, name: Var, lvalue: Lvalue, init_type: Type) -> bool
        self.partial_types[-1].map[name] = lvalue
        return True

+    def is_valid_ordereddict_partial_value_type(self, t: Instance) -> bool:


Add a short docstring with an example (also the name needs fixing, see above).

ilevkivskyi · 2019-12-18T15:09:12Z

mypy/checker.py

+            #       they leak in cases like defaultdict(list) due to a bug.
+            #       This can result in incorrect types being inferred, but only
+            #       in rare cases.
+            if isinstance(arg, (TypeVarType, UninhabitedType)):


Should this also include NoneType? Otherwise it may not work if strict_optional=False (see the check for key type). Add a test for this.

I'll add a test.

ilevkivskyi · 2019-12-18T15:14:10Z

mypy/types.py

+    # the type argument is not final and will be replaced later (it's TypeVarType
+    # or UninhabitedType).
+    #
+    # TODO: The type argument can only be TypeVarType due to leaking type variables


Should we just store an erased instance here? Then we can remove this TODO and only keep the one in checker.py.

A good point -- I'll try this out.

ilevkivskyi · 2019-12-18T15:14:33Z

mypyc/test-data/fixtures/ir.py

@@ -26,6 +26,9 @@ class ellipsis: pass
 # Primitive types are special in generated code.

 class int:
+    @overload
+    def __init__(self) -> None: pass
+    @overload


Why the change to mypyc needed here?

A test case relies on the int() constructor since it uses defaultdict(int).

ilevkivskyi · 2019-12-18T15:18:24Z

test-data/unit/check-inference.test

+from collections import defaultdict
+x = defaultdict(int)
+x[''] = 1
+reveal_type(x) # N: Revealed type is 'collections.defaultdict[builtins.str, builtins.int]'


Add a similar test case where value type is generic, e.g.:

x = defaultdict(list) # No error x['a'] = [1, 2, 3]

and

x = defaultdict(list) # Error here x['a'] = []

JukkaL · 2019-12-18T15:28:40Z

The typeshed change is accidental, I'll revert it.

JukkaL added 4 commits December 18, 2019 12:39

Update test case

481190d

Fix test

35e0eff

Update comments

b823448

JukkaL requested a review from ilevkivskyi December 18, 2019 14:42

ilevkivskyi approved these changes Dec 18, 2019

View reviewed changes

JukkaL added 2 commits December 18, 2019 15:37

Revert typeshed change

b28f6ce

Respond to feedback

21298b1

JukkaL merged commit c1cd529 into master Dec 18, 2019

JukkaL mentioned this pull request Dec 18, 2019

Infer variable type if initializer is an empty collection #1055

Closed

JelleZijlstra deleted the partial-defaultdict branch December 20, 2019 23:13

hauntsaninja mentioned this pull request Jun 17, 2020

Issue with urlencode typeshed annotation starting in mypy 0.780 python/typeshed#4234

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support type inference for defaultdict() #8167

Support type inference for defaultdict() #8167

JukkaL commented Dec 18, 2019

ilevkivskyi left a comment

ilevkivskyi Dec 18, 2019

JukkaL Dec 18, 2019

ilevkivskyi Dec 18, 2019

JukkaL Dec 18, 2019

ilevkivskyi Dec 18, 2019

JukkaL Dec 18, 2019

ilevkivskyi Dec 18, 2019

ilevkivskyi Dec 18, 2019

JukkaL Dec 18, 2019

ilevkivskyi Dec 18, 2019

JukkaL Dec 18, 2019

ilevkivskyi Dec 18, 2019

JukkaL Dec 18, 2019

ilevkivskyi Dec 18, 2019

JukkaL commented Dec 18, 2019

Support type inference for defaultdict() #8167

Support type inference for defaultdict() #8167

Conversation

JukkaL commented Dec 18, 2019

ilevkivskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JukkaL commented Dec 18, 2019