8000 Support `or` in `isinstance` checks by rwbarton · Pull Request #1712 · python/mypy · GitHub
[go: up one dir, main page]
Skip to content

Support or in isinstance checks #1712

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
Jun 27, 2016
Merged

Support or in isinstance checks #1712

merged 2 commits into from
Jun 27, 2016

Conversation

rwbarton
Copy link
Contributor

Fixes #942.

JukkaL
JukkaL reviewed Jun 15, 2016
View reviewed changes
test-data/unit/check-isinstance.test
8000 [builtins fixtures/isinstance.py]
[out]
main: note: In function "h":
main:13: error: Incompatible return value type (got "object", expected "int")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use a # E: comment here to make the test case easier to read?

@rwbarton rwbarton mentioned this pull request Jun 15, 2016
Closed
@rwbarton rwbarton force-pushed the rwbarton-isinstance branch from 172ac66 to d025588 Compare June 16, 2016 02:59
@rwbarton
Copy link
Contributor Author

Done (and rebased).

JukkaL
JukkaL reviewed Jun 17, 2016
View reviewed changes
test-data/unit/check-isinstance.test
flag = 2
def f(x: A) -> int:
if isinstance(x, B) or isinstance(x, C):
return x.flag
< 8000 details class="details-overlay details-reset position-relative d-inline-block"> Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest revealing the type of x here.

@JukkaL
Copy link
Collaborator
JukkaL commented Jun 17, 2016

LGTM! Some minor nits about test cases and comments above.

rwbarton added 2 commits June 22, 2016 12:24
@rwbarton
Refactor combination of conditional maps
9218536 
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.
@rwbarton
Support or in isinstance checks
56ce18d 
Fixes python#942.
@rwbarton rwbarton force-pushed the rwbarton-isinstance branch from d025588 to 56ce18d Compare June 22, 2016 20:47
@rwbarton
Copy link
Contributor Author

Thanks for the thorough review. I had meant to add more documentation about TypeMap actually, and writing these made me notice that the behavior of isinstance(x, T) and isinstance(x, U) was actually nondeterministic. Pushed a new version addressing this and your other comments.

@JukkaL JukkaL merged commit 32062eb into python:master Jun 27, 2016
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.
2 participants
@rwbarton @JukkaL
0