Skip to content

Multiple isinstance checks in if condition #900

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
JukkaL opened this issue Oct 10, 2015 · 2 comments
Closed

Multiple isinstance checks in if condition #900

JukkaL opened this issue Oct 10, 2015 · 2 comments
Labels

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 10, 2015

Mypy should support multiple isinstance checks in a condition:

def f(x: A, y: A) -> None:
    if isinstance(x, B) and isinstance(y, B):
        # Types of both x and y should be B here, but currently only x is B
        ...

class A: pass
class B(A): pass

This is related to #473.

@jhance
Copy link
Collaborator

jhance commented Oct 10, 2015

I think find_isinstance_check should effectively return a tuple of Tuple[Dict[Node, Tuple[Type, Type]], int] where the int is the old kind. My impression is that the result kind, contrary to the documentation, should represent whether the condition is always true whether than rather the isinstance is always true (based on what we do in the and case currently). If thats not true, it would go inside the Dict.

@ecprice
Copy link
Contributor

ecprice commented Oct 10, 2015

My impression is that the result kind, contrary to the documentation, should represent whether the condition is always true whether than rather the isinstance is always true (based on what we do in the and case currently).

Yeah, that's right. I would probably go with Tuple[Dict[Node, Type], Dict[Node, Type], int], for the return type, though.

Actually, it might be better to be Tuple[Optional[Dict[Node, Type]], Optional[Dict[Node, Type]]] with no kind at all -- if a component is None, then that branch of the conditional cannot happen. If it's not None, then you update the binder using the component. This would avoid the need for the ISINSTANCE_ enum entirely.

jhance added a commit to jhance/mypy that referenced this issue Oct 11, 2015
This also squashes a minor weirdness where in order to infer
isinstance for resolution further in an and-expr, we were
parsing for isinstance in the entire expression!

Overall this is pretty self-explanatory. We get rid of the 'kind'
stuff and just return a tuple of optional dicts.

Fixes python#900.
jhance added a commit to jhance/mypy that referenced this issue Oct 11, 2015
This also squashes a minor weirdness where in order to infer
isinstance for resolution further in an and-expr, we were
parsing for isinstance in the entire expression!

Overall this is pretty self-explanatory. We get rid of the 'kind'
stuff and just return a tuple of optional dicts.

Fixes python#900.
jhance added a commit to jhance/mypy that referenced this issue Oct 11, 2015
This also squashes a minor weirdness where in order to infer
isinstance for resolution further in an and-expr, we were
parsing for isinstance in the entire expression!

Overall this is pretty self-explanatory. We get rid of the 'kind'
stuff and just return a tuple of optional dicts.

Fixes python#900.
jhance added a commit to jhance/mypy that referenced this issue Oct 11, 2015
This also squashes a minor weirdness where in order to infer
isinstance for resolution further in an and-expr, we were
parsing for isinstance in the entire expression!

Overall this is pretty self-explanatory. We get rid of the 'kind'
stuff and just return a tuple of optional dicts.

Fixes python#900.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants