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

Incorrect union narrowing in expression with assignments #8128

Closed
elprans opened this issue Dec 10, 2019 · 8 comments
Closed

Incorrect union narrowing in expression with assignments #8128

elprans opened this issue Dec 10, 2019 · 8 comments
Labels
bug mypy got something wrong priority-1-normal

Comments

@elprans
Copy link
Contributor

elprans commented Dec 10, 2019

bug.py:

from __future__ import annotations
from typing import *


def foo() -> Optional[str]:
    return None


if ((f := foo()) is None or f.lower()):
    pass
$ mypy bug.py
bug.py:9: error: Item "None" of "Optional[str]" has no attribute "lower"
Found 1 error in 1 file (checked 1 source file)
@1st1
Copy link
Member

1st1 commented Feb 3, 2020

@JukkaL @ilevkivskyi Jukka, Ivan, sorry for summoning you explicitly, but do you guys think you could take a look at this? This really blocks us on a few things... :(

@emmatyping
Copy link
Collaborator

@1st1 I realize that this could be painful, but for the provided example, a simple refactor to

f = foo()
if (f is None or f.lower()):
    pass

works as expected. If you have another example where it would be difficult or impossible to work around, we can bump the priority.

Also I believe @Michael0x2a has been working on type narrowing the most recently.

@emmatyping emmatyping added bug mypy got something wrong priority-1-normal labels Feb 3, 2020
@elprans
Copy link
Contributor Author

elprans commented Feb 3, 2020

@ethanhs For some background, we have a lot of code that basically does this:

if is_foo(obj):
   obj.method()

Where is_foo is some non-trivial check on the obj type. Naturally, mypy would not understand constructs like this, since, unlike Typescript, it does not support user-defined type assertions. So, one way to handle this is to change is_foo to return Optional[Foo] instead of bool, which would be almost painless using the walrus operator:

if foo := is_foo(obj):
    foo.method()

The workaround you suggest does not work as easily when the conditional is part of some long chain of elif blocks.

@1st1
Copy link
Member

1st1 commented Feb 4, 2020

works as expected. If you have another example where it would be difficult or impossible to work around, we can bump the priority.

Yeah, the workaround is obvious in simple cases. Take a look at @elprans' explanation of how we want to use walrus/mypy in our EdgeDB codebase. Would be amazing if you could bump the priority as having this fixed can simplify our code quite a bit.

@ilevkivskyi
Copy link
Member

Just to double-check, did you try it on master? There is a chance #8258 fixed this.

In any case, unfortunately I will not have time to work on this.

@elprans
Copy link
Contributor Author

elprans commented Feb 7, 2020

@ilevkivskyi #8258 does seem to solve the simple case in #8128 (comment), but not the original issue reported here.

@elprans
Copy link
Contributor Author

elprans commented Mar 18, 2020

This appears to have been fixed in #8458 (at least the original issue).

@emmatyping
Copy link
Collaborator

Confirmed as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong priority-1-normal
Projects
None yet
Development

No branches or pull requests

4 participants