Skip to content

Regression: isinstance(x, AliasOfTuple) is not supported #3068

Closed

Description

@elazarg

Example:

T = (int, int)
x : object
if isinstance(x, T):
    reveal_type(x)  # Revealed type is 'builtins.object'
    x + x  # E: Unsupported left operand type for + ("object")

This was introduced in #2995

Activity

  1. gvanrossum commented on Mar 27, 2017

    @gvanrossum
    Member

    Also @pkch please have a look.

  2. pkch commented on Mar 27, 2017

    @pkch
    Contributor

    Yes, I will fix it. It's not really a type alias though, since (int, int) is not a type, it's a regular python object. A type alias would be

    T = Tuple[int, int]
    x : object
    if isinstance(x, T):
        reveal_type(x)  # Revealed type is 'builtins.object'
        x + x  # E: Unsupported left operand type for + ("object")
    
    

    and I don't think we intend to support that correct?

  3. elazarg commented on Mar 27, 2017

    @elazarg
    ContributorAuthor

    I have used T = (types ...) in the PR precisely in order to avoid testing against union, but also avoid repeating long, possibly changing, list of types. Maybe isinstance(x, Union[simple types]) should be OK?

    This way or another the decision needs to be conscious.

  4. pkch commented on Mar 28, 2017

    @pkch
    Contributor

    I think it's good to keep typing-related stuff separated from regular python code; otherwise the logic becomes very complicated - both for Python developers and for mypy.

    Originally, this separation was very clean: names defined in typing could only be used in annotations. Later, type aliases were introduced, and so typing domain leaked through to the regular python code in the form of assignments. But it was worth it because type aliases are very useful.

    Now, unless I'm missing something, "type concepts" (i.e., names imported from typing and type aliases) can only be used in annotations or on the RHS of assignment expressions where LHS is a simple identifier.

    I think adding more contexts where "type concepts" can be used should be done rarely and with a very strong use case. In particular, I don't think isinstance(x, Union[A,B]) is worth allowing because it's easy to rewrite it as an or statement.

  5. pkch commented on Mar 28, 2017

    @pkch
    Contributor

    Actually, I just realized that there is one more context where "type concepts" can currently be used: in the second argument of isinstance (and issubclass); this is both valid and useful:

    x: Union[List[str], List[int]]
    if isinstance(x, List[str]): ...
    

    Given that, I don't see any problem with supporting more complicated type expressions, if it seems useful (including isinstance(x, Union[A, B]) which isn't currently allowed).

  6. JelleZijlstra commented on Mar 28, 2017

    @JelleZijlstra
    Member

    That code fails at runtime though (TypeError: Parameterized generics cannot be used with class or instance checks), so what's the point of supporting it in mypy?

  7. pkch commented on Mar 28, 2017

    @pkch
    Contributor

    @JelleZijlstra Oh lol riiiiight, my own PR #3040 is supposed to catch this statically :) I forgot.

    So, then I am going back to my earlier suggestion: perhaps it's better not to expand the contexts where "type concepts" are allowed beyond the current (type annotations and RHS of simple assignments).

  8. added a commit that references this issue on Apr 3, 2017
    62d4bc8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions