Repository navigation
Type error in mypy? #2222
Description
During #2221 (which I have little hope to be actually accepted) I ran into this function in binder.py:
def get(self, expr: Union[Expression, Var]) -> Type:
return self._get(expr.literal_hash)The Var case comes from analyze_var_ref() in checkexpr.py.
Well, I couldn't find where Var.literal_hash is initialized to anything other than None, and returning None in the Var case passes all the tests. So I tried to replace it with something like the following:
def get(self, expr: Union[Expression, Var]) -> Type:
if isinstance(expr, Var):
new_expr = NameExpr(expr.name())
else:
new_expr = expr
return self._get(new_expr.literal_hash)I then ran mypy on itself, but it raised many errors. So I ran the test suite, and I got many failures; however, it seems that these failures are due to a more precise typechecking. For example, take testModifyLoopFor4 (simplified):
x = None # type: Union[int, str]
x = 1
if 1:
x = 'a'The test does not say anything bad in this part, but mypy now complains that x is an int and therefore 'a' cannot be assigned into it. It might be too restrictive, but it is my understanding that this is the desired behavior, since "x=1" rebinds the variable into int (it's valid since int is part of the Union) and then it tries to rebind it to str, but now it is not possible.
I am not 100% sure, since I am not 100% sure how the code works and what is the expected behavior, but it looks like a bug - and if so, there are ~40 "type errors" to fix in the source code (although I suspect none of them is an actual error).
So what is the expected behavior? When and how should types rebind?
Activity
elazarg commentedon Oct 6, 2016
Error messages about the source:
rwbarton commentedon Oct 6, 2016
The type of the variable
xitself never changes. (What is known about) the type of the value ofxcan be different at different points in the program, since the value ofxcan change. But the variable always has typeUnion[int, str], so it's always legal to assign astrto it.elazarg commentedon Oct 6, 2016
So, what should happen in the code?
rwbarton commentedon Oct 6, 2016
The
Varcase ofgetshouldn't exist. The only caller is inanalyze_var_refbut that is dead code. The type of a variable reference is actually refined from the binder elsewhere, innarrow_type_from_binder.elazarg commentedon Oct 6, 2016
Ok that makes sense
gvanrossum commentedon Oct 13, 2016
Is there an action item here? Can you update the topic to reflect that? Else please close.
elazarg commentedon Oct 13, 2016
There is, and it is fixed in #2238.
gvanrossum commentedon Oct 20, 2016
If you sent a separate PR that just fixed that it would be quicker to review.
rwbarton commentedon Oct 20, 2016
Plus it boosts your commit count :)
elazarg commentedon Oct 25, 2016
It just happens to be required for the PR I actually wanted.