Skip to content

Isinstance for if expressions #896

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 10, 2015
Merged

Isinstance for if expressions #896

merged 2 commits into from
Oct 10, 2015

Conversation

jhance
Copy link
Collaborator

@jhance jhance commented Oct 9, 2015

jhance added 2 commits October 9, 2015 00:36
@jhance
[1/2] Support 'not isinstance'
6712919 
If we have an expression of the form "not instance"
then we can recurse and just switch the else and if
expressions. We might as well take this opportunity.
@jhance
[2/2] Support isinstance in conditional-if
a4447fc 
We simply check for isinstance when visiting conditionals
and correctly push for the if/else expressions, giving them new types
only for those particular expressions. After that, the type of the
variable being modified returns to its original type.
@o11c
Copy link
Contributor

o11c commented Oct 9, 2015

What about isinstance(obj, A) and isinstance(obj2, B) ?

@jhance
Copy link
Collaborator Author

jhance commented Oct 9, 2015

See comment in find_isinstance_check in checker.py). Its not supported by if-statements as far as I'm aware, so I'm not supporting it for if-expressions. I don't mind tackling this, but its a separate issue.

Adding this would require modifying find_isinstance_check to be smarter and also returning a Dict[Node, Tuple[Type, Type]] or so. Its kind of tricky, because as you can imagine isinstance(obj, A) and isinstance(obj, B) should resolve obj to whichever is a subclass of the other (or be impossible).

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 10, 2015

Yeah, multiple insinstance checks is a separate issue (I added #900 for it).

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 10, 2015

Looks good, thanks for the PR!

JukkaL added a commit that referenced this pull request Oct 10, 2015
@JukkaL
Merge pull request #896 from jhance/if-expr-isinstance
73fc6b1 
Isinstance for if expressions
@JukkaL JukkaL merged commit 73fc6b1 into python:master Oct 10, 2015
@JukkaL JukkaL mentioned this pull request Oct 10, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jhance @o11c @JukkaL