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

if type(var) is class: ... doesn't constrain type as expected #5847

Closed
AndrewUshakov opened this issue Oct 27, 2018 · 6 comments
Closed

if type(var) is class: ... doesn't constrain type as expected #5847

AndrewUshakov opened this issue Oct 27, 2018 · 6 comments
Labels
false-positive mypy gave an error on correct code feature priority-1-normal

Comments

@AndrewUshakov
Copy link

Consider simplified code (mypy 0.641 messages are in comments):

# coding=utf-8

from typing import TYPE_CHECKING


class A:
    def __init__(self, name: str) -> None:
        self.name = name


class B(A):
    def __init__(self, name: str, value: float) -> None:
        super().__init__(name)
        self.value = value


if __name__ == '__main__':

    lst = tuple([A('abc'), B('cde', 2.718), 'abc'])
    if TYPE_CHECKING:
        reveal_type(lst)            # main.py:21: error: Revealed type is 'builtins.tuple[builtins.object*]'

    for x in lst:
        if TYPE_CHECKING:
            reveal_type(x)          # main.py:25: error: Revealed type is 'builtins.object*'
        if type(x) is B:
            if TYPE_CHECKING:
                reveal_type(x)      # main.py:28: error: Revealed type is 'builtins.object*' ??????????
            print(x.name, x.value)  # main.py:29: error: "object" has no attribute "name"    ??????????
                                    # main.py:29: error: "object" has no attribute "value"   ??????????
    print()

    for y in lst:
        if TYPE_CHECKING:
            reveal_type(y)          # main.py:35: error: Revealed type is 'builtins.object*'
        if isinstance(y, B):
            if TYPE_CHECKING:
                reveal_type(y)      # main.py:38: error: Revealed type is 'main.B'
            print(y.name, y.value)

Both loops are equivalent, but mypy complains about line 29 because the "isinstance(x, B)" works but "type(x) is B" doesn't. Situation is slightly similar to the issue #4864.

P.S. In real situation I have more complicated hierarchy of classes (including classes C(B) etc.) and need to constraint processing to objects of type B only.

@gvanrossum
Copy link
Member

Note that mypy doesn't have a way (internally, nor in the type system) to express "B but not a subclass of B". The best we could probably do is make type(x) is B equivalent to isinstance(x, B), but that would be cheating (a little but).

Until this is fixed, as a workaround, you could write type(x) is B and isinstance(x, B).

@AndrewUshakov
Copy link
Author

Thank you. Other variant, use cast(B, x). This trick also works and doesn't require explanations.

Or, may be, it will be better to implement some special type of comment like # type: assume x: B
to help mypy in difficult situations? Such comments will be less general than # type: ignore, which hide all errors in the line.

@gvanrossum
Copy link
Member

The cast works, but I don't like it, because it silences possible errors too. Adding type comments is not recommended. We should probably just add support for type(x) is B. But it'll be low priority given that it's very uncommon and there's a reasonable workaround.

@Tinche
Copy link
Contributor

Tinche commented Nov 20, 2018

I've been adding mypy to my work code base and this is one of the problems I've encountered. (The other one is unions being used in a runtime context error that should be reverted in my opinion.)

This is the pattern I see most often:

  • a microservice servicing different requests
  • I have an attrs class for each request
  • the natural way to model this is having a union of all requests
  • I can create an object from json satisfying this union using cattrs
  • now I need to actually dispatch based on the real class of the request
  • singledispatch works for this, but sometimes I can't use it
  • I would like a single type() call and a chain of its, because chaining isinstance is expensive for a large number of union members

I agree it's not high priority, but I do see it in use. Thanks for the workaround :)

@erictraut
Copy link

This appears to work fine in the latest version of mypy, so I think it can be closed.

@gvanrossum
Copy link
Member

Agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive mypy gave an error on correct code feature priority-1-normal
Projects
None yet
Development

No branches or pull requests

5 participants