Narrow type with typevar when the lower bound of its upper bound and that type is same #10658
Conversation
ethframe
changed the title
mypy/meet.py
Outdated
| @@ -77,6 +78,10 @@ def narrow_declared_type(declared: Type, narrowed: Type) -> Type: | |||
| and narrowed.type.is_metaclass()): | |||
| # We'd need intersection types, so give up. | |||
| return declared | |||
| elif isinstance(narrowed, TypeVarType): | |||
| if is_same_type(narrowed.upper_bound, meet_types(narrowed.upper_bound, declared)): | |||
There was a problem hiding this comment.
Does it work to check is_subtype(narrowed.upper_bound, declared) instead?
Mathematically, the two should be equivalent: x ≤ y if and only if x ∧ y = x (see https://en.wikipedia.org/wiki/Join_and_meet).
In practice, mypy's implementations fall short of mathematical perfection, and I think that is_subtype is more reliable than meet_types, and reads more naturally.
There was a problem hiding this comment.
The difference from is_subtype is how the UnboundType is treated. When type of declared is UnboundType, is_subtype will always return True. But since the type of meet_type result in this case is either AnyType, NoneType or UninhabitedType, is_same_type will result in True only if narrowed.upper_bound is also of AnyType, NoneType, UninhabitedType or UnboundType. I thought it is better not to unconditionally narrow unbound type with type variable.
There was a problem hiding this comment.
Hm. OK, that seems plausible. Do you have an example where we get different behavior between the two? I think that would make it easier for me to consider the difference.
There was a problem hiding this comment.
I tried to come up with an example and while I was skimming through typeanal.py to figure out how one can get an unbound type during type checking phase, I've learned about this issue. Seems that there is no need to consider unbound types in the type checker, unless their presence leads to crashes. And since is_subtype works fine with unbound types, I think it is actually a good idea to rewrite the entire PR code like this:
elif isinstance(narrowed, TypeVarType) and is_subtype(narrowed.upper_bound, declared):
return narrowed
|
Sorry for the delay, merging! |
Description
Fixes #10605
This PR extends type narrowing by a type from a generic argument. This is achieved by handling a specific case while narrowing some type with
TypeVarType: if the lower bound of that type andupper_boundof type variable is the same type asupper_bound, then the type can be narrowed to that type variable.