-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Narrow type with typevar when the lower bound of its upper bound and that type is same #10658
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
mypy/meet.py
Outdated
| # 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 narrowedThere 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.
OK, I agree!
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.
Done.
|
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.