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

Unconstrained type variable combined with isinstance() doesn't seem to work #1539

Closed
gvanrossum opened this issue May 17, 2016 · 8 comments
Closed

Comments

@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented May 17, 2016

Not sure this is a bug or not...

from typing import TypeVar

S = TypeVar('S')

def parenthesize(s: S) -> S:
    if isinstance(s, str):
        return '(' + s + ')'
    elif isinstance(s, bytes):
        return b'(' + s + b')'
    else:
        raise TypeError("That's not a string, dude! It's a %s" % type(s))

This gets errors on both return lines:

demo.py: note: In function "parenthesize":
demo.py:7: error: Incompatible return value type: expected S`-1, got builtins.str
demo.py:9: error: Incompatible return value type: expected S`-1, got builtins.bytes

Why? The isinstance() checks look like they should provide sufficient guards.

@yaseppochi
Copy link

@yaseppochi yaseppochi commented May 17, 2016

I'm guessing that s might be of a non-str, non-bytes type, and mypy doesn't recognize "else: raise" as a type guard. I'm currently in day-job avoidance, so I'd better not go find out by replacing the "raise" with "return s".

@JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented May 17, 2016

The reason the example doesn't work is that after the isinstance() the type of s is str (or bytes), but nothing else changes -- in particular, the return type is still S. str is not a subtype of S, so mypy rejects the return statement.

To make the example work, mypy could perhaps somehow keep track of the fact that S must be a supertype of str after the first isinstance() check, and use this information when checking the return statement.

@gvanrossum
Copy link
Member Author

@gvanrossum gvanrossum commented May 17, 2016

Oh dang. I totally missed that the errors were about the return type, not about the + operators. Oh well. Since I blogged about this and linked to this issue from the blog, you can expect some more drive-by comments.

@rwbarton
Copy link
Contributor

@rwbarton rwbarton commented May 18, 2016

Actually this particular program really is invalid because S might be a subtype of str; then isinstance(s, str) will be true but '(' + s + ')' is still only a str and not an S.

If you change both return statements to return s, though, the program is valid but mypy still rejects it, because it doesn't know how to use both pieces of information simultaneously: that s is of type str and also of type S.

@JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented May 18, 2016

@rwbarton True, I got it wrong. In any case, this is a pretty subtle issue and doesn't look like a high-priority one.

@ddfisher ddfisher added this to the Questions milestone May 19, 2016
@gvanrossum gvanrossum closed this Jan 6, 2017
@jnsnow
Copy link

@jnsnow jnsnow commented Feb 15, 2020

Hi ... I realize this is necromancy, but I'm running into this trying to type some existing code:

        def localize(v: T) -> T:
            if isinstance(v, str):
                return loctable.localize(v, default='')
            return v

The intent is that this function will translate anything it's capable of translating -- a string -- and return everything else untouched. Mypy doesn't like this very much:

export.py:60: error: Incompatible return value type (got "str", expected "T")

... But, of course, T is a str in this case -- but mypy doesn't seem to be able to dynamically constrain typevars.

@JelleZijlstra
Copy link
Collaborator

@JelleZijlstra JelleZijlstra commented Feb 16, 2020

Or v could be a subtype of str, but your loctable.localize would presumably still return a str.

@jnsnow
Copy link

@jnsnow jnsnow commented Feb 22, 2020

That's a good point -- but in this case, localize is typed to always return string because it's performing a lookup and constructing a new object and it doesn't take care to use the type constructor of the type it was given. The annotation here should be correct: localize inflexibly returns a string.

I think using isinstance checks, while unpythonic, is necessary on boundaries between Python and JSON in particular.

Since I arrived here via google, I think a workaround for this lack of type constraining is likely to use something like @overload; I have since used something like the following:

T = TypeVar('T')

@overload
def localize(v: str) -> str: ...

@overload
def localize(v: T) -> T: ...

def localize(v: T) -> Union[str, T]:
    if isinstance(v, str):
        return loctable.detokenize(v, default='')
    return v
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.