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

Constrained TypeVar fails to narrow on left hand side of conditional expression #4134

Open
alanhdu opened this issue Oct 19, 2017 · 16 comments

Comments

@alanhdu
Copy link
Contributor

alanhdu commented Oct 19, 2017

When I run mypy --python-version 3.6 on the following code:

from typing import AnyStr
def f(x: AnyStr) -> bytes:
    return x.encode("ascii") if isinstance(x, str) else x

I get:

test.py:3: error: "bytes" has no attribute "encode"; maybe "decode"?

But as far as I can tell, this is backwards! x.encode("ascii") is only called when isinstance(x, str), not in the else case (where x is a bytes object).

This is with the latest mypy==0.530

@JelleZijlstra
Copy link
Member

I think this is because you're using AnyStr incorrectly. As a TypeVar, it really makes sense only when used multiple times in a signature (or in a few other cases not relevant here). I believe we're going to introduce a warning for incorrect usages like yours, although I can't find the relevant issue now.

Your code should have been written as:

from typing import Union
def f(x: Union[bytes, str]) -> bytes:
    return x.encode("ascii") if isinstance(x, str) else x

which passes mypy with no errors for me.

I don't know why your code gives an error talking about bytes though, and maybe there's a real bug there. I think it's related to the ternary expression, because rewriting the code to use an if-else block removes the error.

@alanhdu
Copy link
Contributor Author

alanhdu commented Oct 19, 2017

Ah... I see. Thanks, Union[bytes, str] works perfectly!

I'll leave this open because I think the inference that x is a bytes object probably represents a bug somewhere:

from typing import Union, Text
def f(x: bytes) -> bytes:
    return x.encode("ascii") if isinstance(x, str) else x

returns the same MyPy error FWIW.

@gvanrossum
Copy link
Member

gvanrossum commented Oct 19, 2017

Hm, I do think there's a bug somewhere, since this gives no errors -- the key difference being that it uses an if statement instead of an if expression:

def f(x: AnyStr) -> bytes:
    if isinstance(x, str):
        return x.encode('ascii')
    else:
        return x

UPDATE: Your second example also points in the same direction -- again if that's rewritten to use a statement it gives no errors (presumably treating the True branch as unreachable code).

@ilevkivskyi
Copy link
Member

I don't think there is any mypy bug here, although this may be a usability problem. What is going on here consists of two parts:

  1. For constrained type variables (like AnyStr) the function body is checked for all constraints (str and bytes in this case).
  2. For ternary expression, mypy can't mark first part as "unreachable", but it doesn't perform any type narrowing either if the types are non-overlapping, this can be seen form following:
x: int
y = reveal_type(x) if isinstance(x, str) else ''  # Revealed type is 'int'

I think the solution here should be also twofold:

  1. Potentially problematic function signatures, like in the original example should get a warning (at least with a note and an exit 0). This is already mentioned.
  2. In addition we should error on attempts of narrowing with non-overlapping types. For user-defined types it is OK to keep current behaviour (there might be custom __instancecheck__ or other dynamic things), but for built-in types - especially for more error-prone situations like str vs bytes vs unicode vs basestring etc. - we should give an error:
# Python 3 mode
x: bytes
...
if isinstance(x, str):  # This should be an error, this is likely a confusion with Python 2
    ...

@gvanrossum
Copy link
Member

I think there are several red herrings in the examples (use of AnyStr for a single argument, or isinstance() with a non-overlapping type, but I still think it's a bug that the code with the ternary expression gives an error while the equivalent code with an if-expression.

def f(x: AnyStr, y: AnyStr) -> bytes:
    x += y
    return x.encode('ascii') if isinstance(x, str) else x  # Error here

def g(x: AnyStr, y: AnyStr) -> bytes:
    x += y
    if isinstance(x, str):
        return x.encode('ascii')  # No error here
    else:
        return x

We used to have more of these, and we solved them by making the binder smarter -- I think there's more of that to be done to make the first version here pass.

@ilevkivskyi
Copy link
Member

@gvanrossum

but I still think it's a bug that the code with the ternary expression gives an error while the equivalent code with an if-expression

This is not easy, since every part of the ternary expression is an expression. It can't just be unreachable like a statement, and should have some type. A possible strategy is to give that "unreachable" expression UninhabitedType (this one has all attributes) and infer a simplified union from ternary expressions, not a join (the second is an independent problem, but I think we already have an issue for this).

@gvanrossum
Copy link
Member

gvanrossum commented Oct 20, 2017 via email

@ilevkivskyi
Copy link
Member

So just to clarify [...]

I think this is exactly what happens.

@petergaultney
Copy link

What would be the appropriate place in the docs to document this behavior? https://mypy.readthedocs.io/en/latest/common_issues.html ? I'll submit a PR (unless I'm just missing where this is already documented!)

@ilevkivskyi
Copy link
Member

@petergaultney This depends on what exactly you want to document (i.e. what is "this behavior").

@petergaultney
Copy link

I think the correct section of the docs would be here: https://mypy.readthedocs.io/en/latest/common_issues.html?highlight=isinstance#complex-type-tests

And the documentation would be something along the lines of "Ternary expressions will not allow type inference in the same way as using if statements."

@ilevkivskyi
Copy link
Member

OK, I see yes, this makes sense (this is not a permanent limitation, but likely to be around for a while). You can make a PR and then we can polish the wording.

@mbdevpl
Copy link

mbdevpl commented Feb 4, 2020

I'm wondering is this issue essentially the same as what I'm getting, or should I submit a new one?

What I have essentially is - types are inferred correctly when using if, but not when using ternary conditional. Same logic.

$ cat conditions.py
def conditional(condition: bool = False) -> tuple:
    if condition:
        return ((1, '', 0),)
    return ()


def inline_conditional(condition: bool = False) -> tuple:
    return ((1, '', 0),) if condition else ()  # line 8 is here
$ mypy conditions.py
conditions.py:8: error: Incompatible return value type (got "object", expected "Tuple[Any, ...]")
Found 1 error in 1 file (checked 1 source file)

My environment:

  • Ubuntu 19.10
  • Python 3.8.0 (default, Oct 23 2019, 16:52:33) [GCC 9.2.1 20191008]
  • mypy 0.761
  • typed-ast 1.4.1

I know this issue revolves around AnyStr, but from my observations, mypy fails to detect multitude of types when they are returned from the ternary conditional operator, not just AnyStr. Example: #6755

Also wording chosen in #6649 concentrates on isinstance(), while it's not present in my example.

@ntn9995
Copy link

ntn9995 commented Jul 2, 2020

I think I've run into the same issue working with requests. Since it looks similar enough to these cases I'll leave it here instead of submitting a new issue.

Specifically, when working with request.Session.

This fails.

def send_get(session: Optional[requests.Session] = None) -> requests.Response:
    fetch = session.get if session is not None else requests.get
    return fetch("https://someurl.com")

While this works.

def send_get(session: Optional[requests.Session] = None) -> requests.Response:
    if session is not None:
        fetch = session.get
    else:
        fetch = requests.get
    return fetch("https://someurl.com")

I'm not sure this is a mypy problem, or a problem specifically with requests.

mypy output:

➜ mypy --show-error-codes request.py
request.py:15: error: Cannot call function of unknown type  [operator]
Found 1 error in 1 file (checked 1 source file)

This is on:

  • mypy 0.782
  • requests 2.24
  • typed-ast 1.4.1
  • python 3.7.7

@saaketp
Copy link

saaketp commented Sep 9, 2021

A very simple example of Ternary expression that fails with mypy.
Also comparing with pyright which doesn't complain and does much better inference.

condition = True
a = [1]
b = [True]
c = a if condition else b

# mypy - Revealed type is "builtins.object"
# pyright - Type of "c" is "list[int] | list[bool]"
reveal_type(c)

# mypy - Value of type "object" is not indexable  [index]
# pyright - No errors
print(c[0])

@KotlinIsland
Copy link
Contributor

The title could be modified to something like "Constrained TypeVar fails to narrow on left hand side of conditional expression"

@hauntsaninja hauntsaninja changed the title Incorrect Type Inference in Ternary Expression Constrained TypeVar fails to narrow on left hand side of conditional expression Aug 19, 2023
@hauntsaninja hauntsaninja removed the topic-join-v-union Using join vs. using unions label Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests