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

cannot narrow some-type|Any to some-type by isinstance #4508

Closed
pinterior opened this issue Jan 23, 2023 · 4 comments
Closed

cannot narrow some-type|Any to some-type by isinstance #4508

pinterior opened this issue Jan 23, 2023 · 4 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@pinterior
Copy link

pinterior commented Jan 23, 2023

Describe the bug

Unions can be narrowed to it's element type by isinstance as described in https://github.com/microsoft/pyright/blob/main/docs/type-concepts.md#type-narrowing .

but, for example Union[str,Any] cannot be narrowed to str by isinstance(..., str).

To Reproduce

from typing import Any, Union
from typing_extensions import reveal_type

def t0() -> Any: ...
def t1() -> str: ...
def t2() -> Union[str, int]: ...
def t3() -> Union[str, Any]: ...
def t4() -> Union[str, int, Any]: ...

r0 = t0()
r1 = t1()
r2 = t2()
r3 = t3()
r4 = t4()

reveal_type(r0)
if isinstance(r0, str):
    reveal_type(r0)

reveal_type(r1)
if isinstance(r1, str):
    reveal_type(r1)

reveal_type(r2)
if isinstance(r2, str):
    reveal_type(r2)

reveal_type(r3)  # str|Any
if isinstance(r3, str):
    reveal_type(r3)  # str expeted, but str|Any

reveal_type(r4)  # str|int|Any
if isinstance(r4, str):
    reveal_type(r4)  # str expeted, but str|Any
  bb.py:16:13 - information: Type of "r0" is "Any"
  bb.py:18:17 - information: Type of "r0" is "str"
  bb.py:20:13 - information: Type of "r1" is "str"
  bb.py:22:17 - information: Type of "r1" is "str"
  bb.py:24:13 - information: Type of "r2" is "str | int"
  bb.py:26:17 - information: Type of "r2" is "str"
  bb.py:28:13 - information: Type of "r3" is "str | Any"
  bb.py:30:17 - information: Type of "r3" is "str | Any"
  bb.py:32:13 - information: Type of "r4" is "str | int | Any"
  bb.py:34:17 - information: Type of "r4" is "str | Any"

Expected behavior

in above example, all reveal_type in if isinstance(..., str): should output str.

VS Code extension or command-line

command-line tool, 1.1.290

@erictraut
Copy link
Collaborator

I agree this is inconsistent. I've updated the isinstance type narrowing logic to be more consistent in this case. This will be included in the next release.

erictraut pushed a commit that referenced this issue Jan 23, 2023
…g the narrowing of a type that contains a union with `Any` or `Unknown`. This addresses #4508.
@erictraut erictraut added bug Something isn't working addressed in next version Issue is fixed and will appear in next published version needs investigation Requires additional investigation to determine course of action and removed addressed in next version Issue is fixed and will appear in next published version labels Jan 23, 2023
@erictraut
Copy link
Collaborator

I needed to back out this change for now because it introduced a regression in some other isinstance narrowing cases. I'll need to see if I can make both cases work.

@erictraut erictraut added addressed in next version Issue is fixed and will appear in next published version and removed needs investigation Requires additional investigation to determine course of action labels Jan 24, 2023
erictraut pushed a commit that referenced this issue Jan 24, 2023
… handling the narrowing of a type that contains a union with `Any` or `Unknown`. This addresses #4508."

This reverts commit dd1e727.
erictraut pushed a commit that referenced this issue Jan 24, 2023
…g the narrowing of a type that contains a union with `Any` or `Unknown`. This addresses #4508.
@erictraut
Copy link
Collaborator

I found a way to make both cases work. This will be included in the next release.

@erictraut
Copy link
Collaborator

This is included in pyright 1.1.291, which I just published. It will also be included in a future release of pylance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants