Skip to content

Narrowing str | dict[str, str] with isinstance and .get() #13351

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

Closed
Akuli opened this issue Aug 7, 2022 · 5 comments
Closed

Narrowing str | dict[str, str] with isinstance and .get() #13351

Akuli opened this issue Aug 7, 2022 · 5 comments
Labels
bug mypy got something wrong topic-type-context Type context / bidirectional inference topic-type-narrowing Conditional type narrowing / binder

Comments

@Akuli
Copy link
Contributor

Akuli commented Aug 7, 2022

Bug Report

To Reproduce

def first() -> None:
    tag_name: str | dict[str, str]
    if isinstance(tag_name, dict):
        tag_name = tag_name.get("foo", "bar")
    reveal_type(tag_name)

def second() -> None:
    tag_name: str | dict[str, str]
    if isinstance(tag_name, dict):
        tag_name = tag_name["foo"]
    reveal_type(tag_name)

Expected Behavior

str
str

Actual Behavior

Union[str, dict[str, str]]
str

Your Environment

latest mypy

@Akuli Akuli added the bug mypy got something wrong label Aug 7, 2022
@AlexWaygood AlexWaygood added the topic-type-narrowing Conditional type narrowing / binder label Aug 7, 2022
@RedKnite5
Copy link

RedKnite5 commented Aug 19, 2022

I've been looking at this and figured out so far that when mypy accepts the CallExpr node Union[builtins.str, builtins.dict[builtins.str, builtins.str]] is added to self.type_context in ExpressionChecker. This is propagated to apply_generic_arguments in applytype.py as context where it is used to deduce the return type of the call to get.

The problem I'm having is that I'm not familiar with how apply_generic_arguments works so I'm not sure where in the function it messes up and what possible solutions would actually totally break its intended function.

@Akuli
Copy link
Contributor Author

Akuli commented Aug 19, 2022

I looked into this a bit too. Here's a more self-contained reproducer:

from __future__ import annotations
from typing import TypeVar

T = TypeVar("T")

class MyDict:
    def get_item(self) -> str: ...
    def get(self, x: T) -> str | T: ...

def first() -> None:
    tag_name: str | MyDict
    if isinstance(tag_name, MyDict):
        tag_name = tag_name.get("asd")
    reveal_type(tag_name)

def second() -> None:
    tag_name: str | MyDict
    if isinstance(tag_name, MyDict):
        tag_name = tag_name.get_item()
    reveal_type(tag_name)

Next I tried:

def third() -> None:
    tag_name: str | MyDict
    if isinstance(tag_name, MyDict):
        reveal_type(tag_name)               # MyDict
        reveal_type(tag_name.get("asd"))    # str
        tag_name = tag_name.get("asd")
        reveal_type(tag_name)               # str | MyDict

When I do reveal_type(tag_name.get("asd")), that happens in a new type context, and works as expected. But when I do tag_name = tag_name.get("asd"), the type of tag_name is taken from the type context as a suggestion for what the type of tag_name.get("asd") might be. To make that happen, mypy resolves T to appropriately.

I think the bug is that str | MyDict is still in the type context by the time we get to the type-narrowed part of the code. When the type narrowing happens, the type of tag_name should become MyDict, and whatever gets str | MyDict into the type context is buggy: it's not looking up the type of the newly-narrowed local variable correctly.

@RedKnite5
Copy link

RedKnite5 commented Aug 19, 2022

I think its about the assignment back to the typed expression. This also exhibits the same behavior and follows a similar data path, but doesn't have type narrowing in the same way.

from typing import TypeVar
T = TypeVar("T")

def func(default: T) -> str | T: ...

def example() -> None:
	tag_name: str | int
	tag_name = func("a")
	reveal_type(tag_name)

Its the union of a type and a generic that turns out to be the same type assigned back to a variable with the union type.
Or maybe I'm misunderstanding how the context stack is supposed to work.

If the type hint on tag_name is omitted:

def example() -> None:
	tag_name = func("a")
	reveal_type(tag_name)

then when it gets to the function infer_function_type_arguments_using_context it returns early without inferring anything and proceeds to get the right answer. So it could be that str | int shouldn't be in the context any more, but I'm not sure.

Edit: Made the example more minimal

@hauntsaninja hauntsaninja added the topic-type-context Type context / bidirectional inference label Aug 19, 2022
@Akuli
Copy link
Contributor Author

Akuli commented Aug 20, 2022

In my example the type of tag_name should be MyDict, not str | MyDict, because type narrowing has happened. While it would be nice for tag_name = func("a") to narrow to str, it's not what this issue is about: here the narrowing has happened already, but the type context isn't picking it up.

Your example, modified to match what I consider to be this issue's scope, would be:

from typing import TypeVar
T = TypeVar("T")

def func(default: T) -> str | T: ...

def example() -> None:
	tag_name: str | int
	if isinstance(tag_name, str):
		tag_name = func("a")
		reveal_type(tag_name)  # str | int :(

@AlexWaygood
Copy link
Member

Fixed in #14151

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-type-context Type context / bidirectional inference topic-type-narrowing Conditional type narrowing / binder
Projects
None yet
Development

No branches or pull requests

4 participants