Skip to content

Get type information from isinstance assertions #1346

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

Merged
merged 2 commits into from
Apr 8, 2016

Conversation

ddfisher
Copy link
Collaborator

@ddfisher ddfisher commented Apr 8, 2016

Fixes #475.

Should probably be reviewed by someone who understands the checker's self.binder.

@gvanrossum
Copy link
Member

LGTM, you an merge it yourself once the tests pass. Yay!

@gvanrossum
Copy link
Member

Hold on, I don't know the binder that well.

--Guido (mobile)
On Apr 7, 2016 5:35 PM, "David Fisher" [email protected] wrote:

Fixes #475 #475.

Should probably be reviewed by someone who understands the checker's

self.binder.

You can view, comment on, or merge this pull request online at:

#1346
Commit Summary

  • Get type information from isinstance assertions

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#1346

@ddfisher
Copy link
Collaborator Author

ddfisher commented Apr 8, 2016

One caveat that I realized is that this will widen the type if you assert something very defensively. For example:

def f(x: int):
  assert isinstance(x, object)
  # x now has type object
  return x + 1  # error

I ran this against our current annotations and found one instance of something like this. (Worked fine otherwise, though!)

@gvanrossum
Copy link
Member

gvanrossum commented Apr 8, 2016 via email

@ddfisher
Copy link
Collaborator Author

ddfisher commented Apr 8, 2016

Just checked -- it has the same behavior. (Which make sense, as I based this code on the if isinstance(...) code.)

@gvanrossum gvanrossum merged commit 848f821 into python:master Apr 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants