-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Always infer in
operator as returning bool
#5688
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
Conversation
Tangentially related: Is there a way to have the test suite do type checks on Mypy's own code? |
@@ -1686,6 +1686,8 @@ def visit_ellipsis(self, e: EllipsisExpr) -> Type: | |||
|
|||
def visit_op_expr(self, e: OpExpr) -> Type: | |||
"""Type check a binary operator expression.""" | |||
if e.op == 'in': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that we won't check type errors within the subexpressions of the in expressions?
Concretely, does mypy still report an error on code like 1 in ([1] + ['x'])
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example you gave still produces an error, but just to be on the safe side, I've added some extra type checking, and a test to make sure this is still an error.
We run a self-check (mypy typechecking itself) as part of the test suite. |
@JelleZijlstra, the reason I asked about type checking Mypy, is that before I submitted this PR, I ran the full test suite with |
That's because CI does a bit more than just run pytest. Check out the contents of You can type-check mypy with itself using something like |
Thanks for that information @gvanrossum. I might suggest adding that to the documentation for tests to run before submitting a PR. |
It’s not the end of the world if CI fails. Just fix it and push again.
On Sun, Sep 30, 2018 at 12:34 PM Joel Croteau ***@***.***> wrote:
Thanks for that information @gvanrossum <https://github.com/gvanrossum>.
I might suggest adding that to the documentation for tests to run before
submitting a PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5688 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACwrMvzerI7bBxF-4SBM8lMyWLgi-jTPks5ugRzegaJpZM4W9wnA>
.
--
--Guido (mobile)
|
I forgot one thing. (It's also not in any README file.) There's a script |
Helps with issue discussed in #5688.
Am I seeing the same behavior with
|
@jonapich
|
🙊 🏃♂️ |
Helps with issue discussed in python#5688.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems solid to me. If you rebase and fix merge conflicts I think we can merge it.
I went and rebased this and will merge it now |
Resolves #5656