FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Isinstance for if expressions by jhance · Pull Request #896 · python/mypy · GitHub

  1. python
  2. mypy

Repository navigation

Isinstance for if expressions - #896

Merged
JukkaL merged 2 commits into
python:masterfrom
jhance:if-expr-isinstance
Oct 10, 2015
ConversationCommits (2)ChecksFiles changed
Merged

Isinstance for if expressions#896
JukkaL merged 2 commits into
python:masterfrom
jhance:if-expr-isinstance

Conversation

jhance commented Oct 9, 2015

Copy link
Copy Markdown
Collaborator

jhance added 2 commits October 9, 2015 00:36
[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.
[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 commented Oct 9, 2015

Copy link
Copy Markdown
Contributor

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

jhance commented Oct 9, 2015

Copy link
Copy Markdown
Collaborator Author

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 commented Oct 10, 2015

Copy link
Copy Markdown
Collaborator

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

JukkaL commented Oct 10, 2015

Copy link
Copy Markdown
Collaborator

Looks good, thanks for the PR!

JukkaL added a commit that referenced this pull request Oct 10, 2015
Merge pull request #896 from jhance/if-expr-isinstance
73fc6b1 
Isinstance for if expressions
JukkaL merged commit 73fc6b1 into python:master Oct 10, 2015
JukkaL mentioned this pull request Oct 10, 2015
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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

Back | FazBrowse Home | New Git URL