-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Generate correct constraints for Unions #1408
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
I hope Jukka can review this one.
|
U = TypeVar('U') | ||
def f(x: Union[T, int], y: T) -> T: pass | ||
f(1, 'a')() # E: "str" not callable | ||
f('a', 1)() # E: "object" not callable |
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.
Also test f('a', 'a')()
and f(1, 1)
.
Ping? |
I've found another issue related to this change in the course of working on #1261. Once my local branch for that project is in a stable state I'll update this PR with the fix. |
The old code always generated a conjunction, where we sometimes want a disjunction. Since we can't represent or solve a disjunction of constraints, need to approximate. In the presence of Unions, the constraints inferred from (template, actual, direction) are not necessarily the reverses of the constraints inferred from (template, actual, neg_op(direction)). The visit_instance and visit_callable cases were updated accordingly. Test testGenericFunctionSubtypingWithUnions fails without this change. Fixes python#1458 and the remaining part of python#1241.
Looks good now -- thanks for sweating the details! Is there an issue for the remaining special case that we still can't handle? No need to fix it now as it would likely require pretty major changes. We can postpone it indefinitely, or at least until users start encountering the issue frequently enough. |
The old code generated a conjunction, where we wanted a disjunction.
Since we can't represent or solve a disjunction of constraints, need
to approximate. Addresses part of #1241.