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

Does not detect type change in two if branches #3803

Closed
StyXman opened this issue Aug 5, 2017 · 9 comments
Closed

Does not detect type change in two if branches #3803

StyXman opened this issue Aug 5, 2017 · 9 comments

Comments

@StyXman
Copy link

StyXman commented Aug 5, 2017

I have code that boils down to this:

from typing import Dict

class Foo:
    pass

my_dict = Dict[Foo, bool]

def bar() -> my_dict:
    return {}

def maybe() -> bool:
    return False

d:my_dict = {}

if True:
    d = bar()
else:
    d[0] = {}  # this assignment is wrong, as 0 is not of type Foo
    reveal_type(d)

If I run mypy on it, I get nothing:

$ mypy mypy_test.py
$ 

Now, if the if reads if False: or even if maybe(), I get:

$ mypy mypy_test.py
mypy_test.py:19: error: Invalid index type "int" for Dict[Foo, bool]; expected type "Foo"
mypy_test.py:19: error: Incompatible types in assignment (expression has type Dict[<nothing>, <nothing>], target has type "bool")
mypy_test.py:20: error: Revealed type is 'builtins.dict[mypy_test.Foo, builtins.bool]'

which is what I was expecting. I think this is more or less related to #1174.

If you're curious, the code where this was silent is this one. Line 260 has the right type, but the construction in the else branch is of type Dict[int, Dict[int, bool]].

@ethanhs
Copy link
Collaborator

ethanhs commented Aug 5, 2017

Mypy by default does look at whether something is always true or always false in an if condition, and will react accordingly. Your if True block means that only d= bar() (a completely valid assignment) is type checked. Thus the empty output is expected.

Are you saying that in your code, mypy is taking the skip variable to incorrectly always be False, thus missing the other branch?

@StyXman
Copy link
Author

StyXman commented Aug 6, 2017

Yes, in my real example, skip can be True or False depending on command line options and some file's existence and creation time. See https://github.com/StyXman/elevation/blob/7340f5e8eecbbe7f28cb4224d2532c2e987057eb/generate_tiles.py#L241 .

@StyXman
Copy link
Author

StyXman commented Aug 6, 2017

I edited the links to code so they point to the right commit, so I can fix it without breaking this report.

@gvanrossum
Copy link
Member

That link is a 404, can you double-check it?

@ethanhs
Copy link
Collaborator

ethanhs commented Aug 7, 2017

The correct link is https://github.com/StyXman/elevation/blob/7340f5e8eecbbe7f28cb4224d2532c2e987057eb/generate_tiles.py#L241 (on Github the URLs are github.com/repo/blob/[commit or "master"]/file)

@gvanrossum
Copy link
Member

OK, but then the bug report seems wrong -- the bug report complains that there's no type checking in a branch that's detectable dead code (due to if True) but the actual code does not have dead code (skip is not statically known to be always True or always False). Perhaps the OP is confused about mypy's magical powers?

@StyXman
Copy link
Author

StyXman commented Aug 7, 2017

Yeah, sorry about the links, I edited them by hand.

the bug report complains that there's no type checking in a branch that's detectable dead code (due to if True) but the actual code does not have dead code (skip is not statically known to be always True or always False).

No, my complaint is that skip is undecidable without running, render_children is given a explicit type (https://github.com/StyXman/elevation/blob/7340f5e8eecbbe7f28cb4224d2532c2e987057eb/generate_tiles.py#L258), and mypy does not detect that the assignments in the else branch break the defined type. Probably my attempt to give a simplified version of the code was misleading, sorry.

@gvanrossum
Copy link
Member

Then I suspect it's all because this all inside the loop() function which is unannotated. Per PEP 484, mypy doesn't report errors inside unannotated functions. All you have to do is change that to

def loop(self) -> None:

@ilevkivskyi
Copy link
Member

It looks like no actions required here.

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

No branches or pull requests

4 participants