Make Type[T] comparisons work #1806
Conversation
This commit introduces a workaround to fix the bug discussed in python#1787 Previously, code where you compared two types (eg `int == int`) would cause mypy to incorrectly report a "too few arguments" error.
|
@JukkaL What do you think of this? It works but it feels a bit ad-hoc. The same behavior applies to all operators (everything in nodes.op_methods and nearby variables) but it feels the key problem is happening because of some confusion between the class and the metaclass (or between the instance and the class) -- it's just only apparent for |
|
More background: for operators, the method name (in this case |
|
Yeah, this feels a little ad hoc. It seems that |
This commit introduces a workaround to fix the bug discussed in python#1787 Previously, code where you compared two types (eg `int == int`) would cause mypy to incorrectly report a "too few arguments" error.
After talking with Guido, I realized Python special-cases literally the operators (==, +, etc), NOT the magic methods themselves. So, that means we need to special-case "a == b", but not "a.__eq__(b)". As a result, checking to see if the method name was a magic method or not was the wrong thing to do -- if the name of the method is "__eq__", there's no way to know if the code was originally "foo == bar" or "foo.__eq__(bar)". So, rather then trying to do something with the operator name, I can instead check and see if the node is an OpExpr or ComparisonExpr node vs a CallExpr node.
…o fix-type-comparison
| builtin_type, not_ready_callback, msg) | ||
| if result: | ||
| return result | ||
| if not isinstance(node, (OpExpr, ComparisonExpr)): |
There was a problem hiding this comment.
node is just used as a context for errors, and isn't necessarily reliable. I don't think it probably shouldn't be used for this purpose.
There was a problem hiding this comment.
Also, this doesn't catch all cases, e.g. __getitem__.
There was a problem hiding this comment.
I'd consider adding an optional bool parameter to this function instead and setting it in check_op (in checkexpr.py).
|
Can you fix the problems surfaced by CI? This looks good otherwise. |
|
Oh, whoops -- I forgot to test after merging. Doing that now. |
This commit introduces a workaround to fix the bug discussed in #1787
Previously, code where you compared two types (eg
int == intorint != int) would cause mypy to incorrectly report a "too few arguments" error.