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

Infer variable type if initializer is an empty collection #1055

Closed
JukkaL opened this issue Dec 6, 2015 · 12 comments
Closed

Infer variable type if initializer is an empty collection #1055

JukkaL opened this issue Dec 6, 2015 · 12 comments
Labels
false-positive mypy gave an error on correct code feature priority-1-normal

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Dec 6, 2015

Code like this is one of the most common cases where we need a variable annotation (DONE):

def f() -> None:
    x = []   # mypy insists on annotation (such as List[int]) here
    for i in range(5):
        x.append(i * 2)

Mypy could do a better job of inferring types in cases like these:

  • Variable is initialized with an empty collection (or more generally, a generic type where the type argument values are indeterminate using only local statement context).
  • The first reference to the variable after initialization (in the same scope) mutates the collection and allows us to infer the item types.

These cases would still require an annotation:

  • Collection is read after initialization and before mutation (this happens sometimes but not too often).
  • The variable is not mutated in the scope that initializes it. For example, an attribute is initialized in __init__ and mutated in another method.

This is related to #254.

More common examples where this should work:

(1) DONE

x = {}
x['key'] = 2

(2) DONE

x = set()
x.add(2)

Additional, less common examples (not done):

(3) [postponed -- too rare and complicated]

x = {}
x.setdefault('key', []).append(1)   # this is harder than most examples

(4) DONE

x = {}
x.update({'x': 1})

(5) [rejected -- too rare]

x = set()
x |= {1, 2}

(6) DONE (#8036)

if cond():
    x = []
else:
    x = [1]

(7) DONE (#8167 and #8039)

Similar to above but with a user-defined generic type or a standard library generic type that is not list, dict or set.

Update: After some more analysis, only defaultdict and OrderedDict seem common enough to support.

Not sure about this:

(8) [rejected idea]

x = []
foo(x)  # Should we infer type of x from the argument type? Probably not.
@refi64
Copy link
Contributor

refi64 commented Dec 6, 2015

@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 6, 2015

We'll use a simpler form of type inference that doesn't slow things much (if at all). We aren't going to do whole-program type inference.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 6, 2015

Here's the general idea for how this would work:

  • If we detect an assignment such as x = [] (and we haven't seen x before) we'd initially give x a 'partial' type List[?].
  • When you perform a mutation operation such as append, we infer the missing bits for the type and replace the type with a precise type (for example, List[int]).
  • If you try to do anything else on a variable with a partial type, such as reading from it, mypy would require you to add an annotation to the initialization statement, similar to the current behavior.
  • If there are any variables with partial types when leaving a scope, mypy will ask for an annotation.

JukkaL added a commit that referenced this issue Dec 7, 2015
@JukkaL JukkaL removed the priority label Dec 7, 2015
@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 7, 2015

Cases (1), (2) and list append now work. The remaining cases are less important.

@gvanrossum
Copy link
Member

Awesome! This got rid of 18 out of 31 occurrences of this message. The
remaining occurrences are:

  • set/frozenset
  • None
  • instance variables

On Sun, Dec 6, 2015 at 5:00 PM, Jukka Lehtosalo [email protected]
wrote:

Cases (1), (2) and list append now work. The remaining cases are less
important.


Reply to this email directly or view it on GitHub
#1055 (comment).

--Guido van Rossum (python.org/~guido)

@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 7, 2015

None shouldn't be hard. Mypy probably won't infer instance variables, except maybe for cases where you populate the collection in the same same place where initialization happens.

There is minimal set support but not sure how far it's practical to take it.

@gvanrossum
Copy link
Member

Do you support discard() for sets? That's one of the cases.
One of the cases found a bug! (Set has no attribute "append")
One set case is complex (the first place where the variable occurs it is
set to a frozen set, but in a different branch it is initialized to a set
and then filled with a loop).
All in all pretty amazing.

On Sun, Dec 6, 2015 at 6:19 PM, Jukka Lehtosalo [email protected]
wrote:

None shouldn't be hard. Mypy probably won't infer instance variables,
except maybe for cases where you populate the collection in the same sample
place where initialization happens.

There is minimal set support but not sure how far it's practical to take
it.


Reply to this email directly or view it on GitHub
#1055 (comment).

--Guido van Rossum (python.org/~guido)

@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 7, 2015

discard() is not supported but it would be pretty straightforward to add. However, I'd rather try to first implement a more general solution that would work with arbitrary methods instead of special casing additional methods.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 31, 2015

extend of list is another common method that is now supported, similar to append.

@pjenvey
Copy link
Member

pjenvey commented Nov 24, 2016

This form currently isn't inferred, can it be?

class A:
    def f(self) -> None:
        self.x = []
        for i in range(5):
            self.x.append(i * 2)

@JukkaL
Copy link
Collaborator Author

JukkaL commented Nov 24, 2016

@pjenvey That's correct, this doesn't work for instance attributes.

PeterJCLaw added a commit to sourcebots/runusb that referenced this issue Jan 7, 2018
This statement is valid; since the list is empty it doesn't really
matter what type of list it is -- update_filesystems is only every
going to try to iterate over it anyway (which will yield no entries).

However, mypy doesn't understand it. Unfortunately I can't find
either a reference to the canonical solution or bug; the nearest
I get is point (8) at python/mypy#1055.
@JukkaL JukkaL added false-positive mypy gave an error on correct code priority-1-normal labels May 18, 2018
JukkaL added a commit that referenced this issue Nov 29, 2019
Code like this no longer requires a type annotation:

```
a = []
if foo():
    a = [1]
```

Work towards #1055.
JukkaL added a commit that referenced this issue Nov 29, 2019
This example no longer needs a type annotation:

```
from collections import OrderedDict

x = OrderedDict()
x[1] = 'x'
```

Work towards #1055.
ilevkivskyi added a commit that referenced this issue Dec 3, 2019
Work towards #1055

Currently partial types are supported for local variables. However, only partial `None` types are supported for `self` attributes. This PR adds the same level of support to generic partial types. They follow mostly the same rules:
* A partial type can be refined in the _same_ method where it is defined.
* But a partial type from class body can not be refined in a method, as if `local_partial_types = True`.

The logic is pretty simple: the `.node` attribute for `self.attr` expressions is set to `None`, so I added a little helper to get it from the class symbol table instead.
@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 18, 2019

I think that this works pretty well now after the latest improvements, including #8167. There are some possible refinements but they are nice to haves.

@JukkaL JukkaL closed this as completed Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive mypy gave an error on correct code feature priority-1-normal
Projects
None yet
Development

No branches or pull requests

4 participants