Skip to content
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

Mypy fails on Union[float, int] + Union[float, int] #2128

Closed
rowillia opened this issue Sep 13, 2016 · 8 comments · Fixed by #5545
Closed

Mypy fails on Union[float, int] + Union[float, int] #2128

rowillia opened this issue Sep 13, 2016 · 8 comments · Fixed by #5545
Assignees
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-union-types

Comments

@rowillia
Copy link
Contributor

def add_value(a: Union[float, int], b: Union[float, int]) -> Union[float, int]:
    return a + b

Results in:

error: Unsupported operand types for + ("Union[float, int]" and "int")

Interestingly both

def add_value(a: Union[float, int], b: float) -> Union[float, int]:
    return a + b

and

def add_value(a: int, b: Union[float, int]) -> Union[float, int]:
    return a + b

work fine

@gvanrossum
Copy link
Member

That's definitely problematic. I suspect it's because we don't try all combinations of all unions involved in an expression. We should probably do better, although overuse of this could cause performance regressions.

@elazarg
Copy link
Contributor

elazarg commented Sep 20, 2016

Now I believe that this is another incarnation of overload python/typing#253 and therefore #1941 and the others mentioned there. I cannot base this claim though.

@wsanchez
Copy link

Regarding @gvanrossum's comment: "overuse of this could cause performance regressions"…

Uh… code that's wrong isn't especially useful no matter how fast it is.

If it's too slow for whatever criteria, then it would be good to make this case a warning rather than an error, as false positives break CI builds that are trying to use mypy for code quality checks, and this check is clearly incomplete/broken.

@wsanchez
Copy link

(or give up with a warning if the matrix is too large)

@stefanuddenberg
Copy link

I've encountered this bug most recently in version 0.610. A fix would be much appreciated!

@ilevkivskyi
Copy link
Member

Here is my diagnosis, mypy first asks "What is the signature of __add__ when accessed on Union[int, float]?" Looking at typeshed, this results in Union[int -> int, float -> float], then mypy typechecks the call Union[int -> int, float -> float](Union[int, float]), and correctly rejects it. It then falls back to __radd__ with obviously still no success. The flaw in this logic is that __add__ to __radd__ fallback happens at runtime, and to match this we need to allow fallback for some elements of a union. So the fix would be straightforward, first check float.__add__(Union[int, float]), be satisfied, then check int.__add__(Union[int, float]) and fallback to __radd__. @gvanrossum I don't think this will have any measurable performance implications.

Off-topic: it is sad to see many issues that have straightforward fixes, but we just don't have time to implement them, sigh.

@davidlowryduda
Copy link

I have also just encountered this bug (or rather, an equivalent bug concerning Union[int, complex] + Union[int, complex].

@Michael0x2a
Copy link
Collaborator

Assigning myself -- I'm planning on making some fixes to operators in the near future, so I might as well look at this one too.

@ilevkivskyi ilevkivskyi mentioned this issue Aug 29, 2018
12 tasks
Michael0x2a added a commit to Michael0x2a/mypy that referenced this issue Aug 31, 2018
This pull request resolves python#2128 --
it modifies how we check operators to add support for operations like
`Union[int, float] + Union[int, float]`.

This approach basically iterates over all possible variations of the
left and right operands when they're unions and uses the union of the
resulting inferred type as the type of the overall expression.

Some implementation notes:

1.  I attempting "destructuring" just the left operand, which is
    basically the approach proposed here: python#2128 (comment)

    Unfortunately, I discovered it became necessary to also destructure
    the right operand to handle certain edge cases -- see the
    testOperatorDoubleUnionInterwovenUnionAdd test case.

2.  This algorithm varies slightly from what we do for union math
    in that we don't attempt to "preserve" the union/we always
    destructure both operands.

    I'm fairly confident that this is type-safe; I plan on testing
    this pull request against some internal code bases to help
    us gain more confidence.
ilevkivskyi pushed a commit that referenced this issue Sep 5, 2018
* Add support for operators with union operands

This pull request resolves #2128 --
it modifies how we check operators to add support for operations like
`Union[int, float] + Union[int, float]`.

This approach basically iterates over all possible variations of the
left and right operands when they're unions and uses the union of the
resulting inferred type as the type of the overall expression.

Some implementation notes:

1.  I attempting "destructuring" just the left operand, which is
    basically the approach proposed here: #2128 (comment)

    Unfortunately, I discovered it became necessary to also destructure
    the right operand to handle certain edge cases -- see the
    testOperatorDoubleUnionInterwovenUnionAdd test case.

2.  This algorithm varies slightly from what we do for union math
    in that we don't attempt to "preserve" the union/we always
    destructure both operands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-union-types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants