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

Cannot inference common base class when root of hierarchy is Any #5134

Closed
asottile opened this issue Jun 1, 2018 · 6 comments
Closed

Cannot inference common base class when root of hierarchy is Any #5134

asottile opened this issue Jun 1, 2018 · 6 comments
Labels
bug mypy got something wrong topic-join-v-union Using join vs. using unions topic-ternary-expression a if b else c

Comments

@asottile
Copy link
Contributor

asottile commented Jun 1, 2018

I believe this to be a bug

code

(this has been boiled down from a more complete example)

from typing import Type

from external import ExtBase


class Base(ExtBase):
    pass


class Concrete1(Base):
    pass


class Concrete2(Concrete1):
    pass


def f(x: Type[Base]) -> None:
    pass


def g(x: bool) -> None:
    #reveal_type(Base)
    #reveal_type(Concrete1)
    #reveal_type(Concrete2)
    #reveal_type(Concrete1 if x else Concrete2)
    f(Concrete1 if x else Concrete2)

output

$  mypy t2.py --ignore-missing-imports
t2.py:27: error: Argument 1 to "f" has incompatible type "Type[object]"; expected "Type[Base]"

output with types revealed

$ mypy t2.py --ignore-missing-imports
t2.py:23: error: Revealed type is 'def (*_args: Any, **_kwds: Any) -> t2.Base'
t2.py:24: error: Revealed type is 'def (*_args: Any, **_kwds: Any) -> t2.Concrete1'
t2.py:25: error: Revealed type is 'def (*_args: Any, **_kwds: Any) -> t2.Concrete2'
t2.py:26: error: Revealed type is 'def (*_args: Any, **_kwds: Any) -> builtins.object'
t2.py:27: error: Argument 1 to "f" has incompatible type "Type[object]"; expected "Type[Base]"

version information

$ mypy --version
mypy 0.600

against master

same error

peculiarly working

diff --git a/t2.py b/t2.py
index 91a8309..6f35532 100644
--- a/t2.py
+++ b/t2.py
@@ -24,4 +24,8 @@ def g(x: bool) -> None:
     #reveal_type(Concrete1)
     #reveal_type(Concrete2)
     #reveal_type(Concrete1 if x else Concrete2)
-    f(Concrete1 if x else Concrete2)
+    if x:
+        t = Concrete1
+    else:
+        t = Concrete2
+    f(t)
@asottile
Copy link
Contributor Author

asottile commented Jun 1, 2018

Note that this also passes (why I believe to be ~related to Any here):

diff --git a/t2.py b/t2.py
index 91a8309..1e9896b 100644
--- a/t2.py
+++ b/t2.py
@@ -3,7 +3,7 @@ from typing import Type
 from external import ExtBase
 
 
-class Base(ExtBase):
+class Base:
     pass
 
 

@gvanrossum
Copy link
Member

This looks like another case where the conditional expression should take the context into account. I wonder if it works as expected when #5095 is reverted?

@asottile
Copy link
Contributor Author

asottile commented Jun 1, 2018

Seems to work as expected when reverting that, yes:

$ git -C mypy/ show | head -6
commit 3e529fc7655266bec48044aaa71acf4df203c72d
Author: Anthony Sottile <[email protected]>
Date:   Fri Jun 1 15:42:13 2018 -0700

    Revert "Don't always infer unions for conditional expressions (#5095)"
    
$ mypy --ignore-missing-imports t.py 
$

$ # with reveal
$ mypy --ignore-missing-imports t.py 
t.py:23: error: Revealed type is 'def (*_args: Any, **_kwds: Any) -> t.Base'
t.py:24: error: Revealed type is 'def (*_args: Any, **_kwds: Any) -> t.Concrete1'
t.py:25: error: Revealed type is 'def (*_args: Any, **_kwds: Any) -> t.Concrete2'
t.py:26: error: Revealed type is 'def (*_args: Any, **_kwds: Any) -> t.Concrete1'

@gvanrossum
Copy link
Member

OK, then the fix would entail improving on that diff (it partially reverted #5041, which would have the desired effect here but not in general). Perhaps we should always try the make_simplified_union() call and accept the return value if it's not a Union -- that seems to be the case here.

@ilevkivskyi
Copy link
Member

TBH, I would also allow Optional[...] as a result of make_simplified_union, so that the rules would be:

  • in union context, always use union
  • otherwise, use make_simplified_union if the result is a non-union, or a union of two types one of which is None
  • otherwise fallback to join

The point is that Any can "swallow" None, so that unimported if x else None results in just Any, not Optional[Any] (this however reveals few dozens of real errors in our internal codebases).

@hauntsaninja
Copy link
Collaborator

I fixed this in #14404

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-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

5 participants