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

mypy infers conditional expression of Dict[str, object] to be object instead of union #7835

Closed
scascketta opened this issue Oct 31, 2019 · 5 comments
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-join-v-union Using join vs. using unions topic-ternary-expression a if b else c

Comments

@scascketta
Copy link

  • Are you reporting a bug, or opening a feature request?

Bug

  • Please insert below the code you are checking with mypy,
    or a mock-up repro if the source is private.
from typing import Dict
from dataclasses import dataclass


@dataclass
class Foo:
    bar: bool


@dataclass
class Bar:
    baz: bool


foo_registry: Dict[str, Foo] = {}
bar_registry: Dict[str, Bar] = {}

use_foo = True

registry = foo_registry if use_foo else bar_registry
registry.get('asdfsdf')
  • What is the actual behavior/output?

On the last line, mypy infers that registry has the type object

error: "object" has no attribute "get"
  • What is the behavior/output you expect?

mypy infers that the type of registry to be Union[Dict[str, Foo], Dict[str, Bar]] but it looks like this behavior may have been undone by #5095 ?

  • What are the versions of mypy and Python you are using?

mypy 0.740 and Python 3.7.2

Do you see the same issue after installing mypy from Git master?

I ran into other issues doing this, but I am on the latest release.

  • What are the mypy flags you are using? (For example --strict-optional)

I'm using --strict --warn-unreachable though this occurs with no flags at all.

Thanks for reading.

@JukkaL JukkaL added bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal labels Nov 1, 2019
@JukkaL
Copy link
Collaborator

JukkaL commented Nov 1, 2019

Yeah, it would be better if the inferred type for registry would be a union. However, even Mapping[str, object] would be significantly better than object.

@TH3CHARLie
Copy link
Collaborator

Since I worked on #7780 before, I believe the problem is located in the code snippet below

mypy/mypy/checkexpr.py

Lines 3567 to 3571 in 3b5a62e

# TODO: Always create a union or at least in more cases?
if isinstance(get_proper_type(self.type_context[-1]), UnionType):
res = make_simplified_union([if_type, full_context_else_type])
else:
res = join.join_types(if_type, else_type)

There's no explicit type annotation to declare registry as a Union, so mypy takes the else branch here and simply join the two types from cond expr into an object type
A simple workaround for @scascketta 's case would be:

from typing import Dict
from dataclasses import dataclass


@dataclass
class Foo:
    bar: bool


@dataclass
class Bar:
    baz: bool


foo_registry: Dict[str, Foo] = {}
bar_registry: Dict[str, Bar] = {}

use_foo = True
# annotate registry
registry: Union[Dict[str, Foo], Dict[str, Bar]] = foo_registry if use_foo else bar_registry
registry.get('asdfsdf')

Interesting enough that the TODO in the code has questioned about more Union types, so maybe we should look into that.

@crusaderky
Copy link
Contributor

crusaderky commented Dec 11, 2019

I believe I stumbled on this while creating a list of heterogeneous dicts:

a = {1: {1: 2}}
b = {1: {1: 3}}
c = {2: {1: "x"}}
d = [a, b]
e = [a, c]
reveal_type(a)  # builtins.dict[builtins.int*, builtins.dict*[builtins.int*, builtins.int*]]
reveal_type(b)  # builtins.dict[builtins.int*, builtins.dict*[builtins.int*, builtins.int*]]
reveal_type(c)  # builtins.dict[builtins.int*, builtins.dict*[builtins.int*, builtins.str*]]
reveal_type(d)  # builtins.list[builtins.dict*[builtins.int, builtins.dict[builtins.int, builtins.int]]]
reveal_type(e)  # builtins.list[builtins.object*]

Could you confirm it's the same issue? e.g. is the type of [a, c] inferred using the same algorithm as the type of a if a else c?

It's also interesting to note that a or c works fine.

@babolivier
Copy link

I think I've stumbled upon this issue as well with the following code:

            res = await maybe_awaitable(callback(event))

Where callback is defined as Callable[["synapse.events.EventBase"], Union[bool, str]], and maybe_awaitable as

def maybe_awaitable(value: Union[Awaitable[R], R]) -> Awaitable[R]:

(R is defined as R = TypeVar("R"))

mypy incorrectly identifies the type of res as builtins.object* instead of Union[bool, str]. When I break it down:

r1 = callback(event)
reveal_type(r1)
a = maybe_awaitable(r1)
reveal_type(a)
res = await a
reveal_type(res)

mypy outputs the following:

synapse/events/spamcheck.py:206: note: Revealed type is 'Union[builtins.bool, builtins.str]'
synapse/events/spamcheck.py:208: note: Revealed type is 'typing.Awaitable[builtins.object*]'
synapse/events/spamcheck.py:210: note: Revealed type is 'builtins.object*'

So it looks like this bug is causing mypy to assume the wrong return type for maybe_awaitable(r1) (unless I'm misunderstanding things).

@hauntsaninja
Copy link
Collaborator

Fixed by #17427

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-join-v-union Using join vs. using unions topic-ternary-expression a if b else c
Projects
None yet
Development

No branches or pull requests

8 participants