Skip to content

Unexpected type 'unicode' for os.path.join() with (Any, str) args #5232

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

Closed
JukkaL opened this issue Jun 18, 2018 · 4 comments
Closed

Unexpected type 'unicode' for os.path.join() with (Any, str) args #5232

JukkaL opened this issue Jun 18, 2018 · 4 comments
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-0-high topic-overloads

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 18, 2018

The revealed type for the os.path.join call in the program below is unicode, even though I'd expect either Any or str, as the arguments have types Any and str:

import os
def f(): pass
reveal_type(os.path.join(f(), 'x'))  # builtins.unicode

Here's a self-contained example:

# a.pyi
from typing import overload
@overload
def join(__p1: bytes, *p: bytes) -> bytes: ...
@overload
def join(__p1: unicode, *p: unicode) -> unicode: ...
# program.py
from a import join
def f(): pass
reveal_type(join(f(), 'x'))  # builtins.unicode

Run as mypy -2 program.py.

@Michael0x2a Do you have time to have a look at this? This is causing issues in internal Dropbox repos. If you are busy, I can try to fix this. This was apparently introduced by f61c2ba.

@JukkaL JukkaL added bug mypy got something wrong priority-0-high false-positive mypy gave an error on correct code topic-overloads labels Jun 18, 2018
@ilevkivskyi
Copy link
Member

I am looking at this now.

@Michael0x2a
Copy link
Collaborator

I think the issue is that due to the way we handle promotions in mypy, bytes is actually technically considered to be a subtype of unicode -- so the union math logic concludes that it's valid to return unicode instead of Any since the former is more precise.

Not sure what the best way to handle this is -- I'd personally prefer just removing that promotion altogether, but that's a very invasive and difficult-to-implement change. Alternatively, we could special-case this in the union math somehow, but that would make it inconsistent with the rest of mypy. E.g. this currently typechecks, for example:

def expects_unicode(x):
    # type: (unicode) -> None
    pass

b = b'foo'  # type: bytes

expects_unicode(b)

@Michael0x2a
Copy link
Collaborator

I guess other options include:

  1. Modifying the "is the last return type a supertype of the previous ones" checks so promotions are disabled altogether.
  2. Removing that check altogether so we always return Any.

@ilevkivskyi
Copy link
Member

My original motivation for requesting it was that this example should work

@overload
def f(x: List[int]) -> List[int]: ...
@overload
def f(x: List[Any]) -> List[Any]: ...

arg: Any
f(arg)  # should be List[Any], _not_ just Any

Also now I think that we should really check both ways, because the logic is that replacing a precise type with Any should ideally not cause new errors. For example:

def f() -> int: ...

@overload
def g(x: int) -> int: ...
@overload
def g(x: float) -> float: ...

x: int = g(f())  # this passes, but will fail if I remove the annotation from 'f'

I will now prepare a PR to return last type only if it is compatible both ways with all other return types (like in the first example with List[Any]).

ilevkivskyi added a commit that referenced this issue Jun 18, 2018
Fixes #5232

This also makes few minor amendments to pythoneval tests.
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-0-high topic-overloads
Projects
None yet
Development

No branches or pull requests

3 participants