Skip to content

Support or in isinstance expressions. #942

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
jhance opened this issue Oct 17, 2015 · 4 comments
Closed

Support or in isinstance expressions. #942

jhance opened this issue Oct 17, 2015 · 4 comments
Assignees
Labels

Comments

@jhance
Copy link
Collaborator

jhance commented Oct 17, 2015

I saw in some place in mypy something like

class A:
    pass

class B(A):
    pass

class C(A):
    pass

x = C()  # type: A
if isinstance(x, B) or isinstance(x, C):
    y = cast(Union[B, C], x)
    ...

We should be able to infer Union[B, C] as the type of x within this expression.

@JukkaL JukkaL added the feature label Oct 17, 2015
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 17, 2015

Yes, that would be nice. #947 is related (tuple as the second argument to isinstance).

@jhance
Copy link
Collaborator Author

jhance commented Oct 18, 2015

It seems like this can get complex pretty quickly (imagine using tuples inside an and, now you effectively have an intersection of unions). Theres so many cases to cover that I'm thinking about just going all-out here and writing an all-encompassing solution.

I've been toying with converting the expressions to DNF and seeing if we can just do an all-encompassing solution that way but I'm not confident in its correctness since it effectively makes complicated cases just as difficult as simple cases.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 18, 2015

At this point it's okay to add some limitations, such as for and all the operands must target different variables. So this wouldn't work correctly:

if isinstance(x, A) and isinstance(x, B): ...

(we'd only recognize the first isinstance, since mypy doesn't support intersection types)

We can revisit this later, but the complex cases are going to be pretty marginal so it's not worth it to complicate everything because of them.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 18, 2015

In general, if in doubt whether a particular idiom or construct is worth supporting, try to find actual examples of that in some existing projects. If you can't find any (or just find a few isolated examples), it's probably not that important, and there are lot of things that mypy can't deal with that we know to be important (such as every issue tagged with 'priority').

rwbarton added a commit to rwbarton/mypy that referenced this issue Jun 14, 2016
Note: This is not the same as python#942. It does address a pattern which
occurs at least once in mypy itself though:
```
if res and (not isinstance(res, Instance) or cast(Instance, res).args):
```
@rwbarton rwbarton self-assigned this Jun 15, 2016
rwbarton added a commit to rwbarton/mypy that referenced this issue Jun 15, 2016
rwbarton added a commit to rwbarton/mypy that referenced this issue Jun 16, 2016
JukkaL pushed a commit that referenced this issue Jun 17, 2016
Note: This is not the same as #942. It does address a pattern which
occurs at least once in mypy itself though:
```
if res and (not isinstance(res, Instance) or cast(Instance, res).args):
```
rwbarton added a commit to rwbarton/mypy that referenced this issue Jun 22, 2016
JukkaL pushed a commit that referenced this issue Jun 27, 2016
There was also a slight logic error in the old version (when
left_if_vars was None but right_if_vars was not, the branch was
treated as reachable). Fix it and add a test for that case.

Fixes #942.
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