Skip to content

explicit x is None should narrow union candidates like isinstance(x, type(None)) #4520

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
bwo opened this issue Jan 29, 2018 · 3 comments
Closed

Comments

@bwo
Copy link
Contributor

bwo commented Jan 29, 2018

from typing import Optional, List  # noqa


def union_test1(x):
    # type: (Optional[List[int]]) -> Optional[int]
    if x is None:
        return x
    else:
        return len(x)


def union_test2(x):
    # type: (Optional[List[int]]) -> Optional[int]
    if isinstance(x, type(None)):
        return x
    else:
        return len(x)

union_test1 with default options gives the error Incompatible return value type (got "Optional[List[int]]", expected "Optional[int]") (with --strict-optional, it does not give this error). union_test2 typechecks just fine. This strikes me as highly counterintuitive; even though (as the docs say) None is a valid value for each type, if one of the above checks allows the typechecker to learn that x is None and therefore not a list, it's hard to see why the other doesn't.

Failing that, I would suggest at least a documentation change to point out both the gotcha embodied in union_test1 and the workaround in union_test2.

@gvanrossum
Copy link
Member

Agreed, I don't see why the first one isn't treated the same as the second.

@ilevkivskyi
Copy link
Member

Agreed, I don't see why the first one isn't treated the same as the second.

This is not the first time I see weird behaviour of None without --strict-optional.

@msullivan
Copy link
Collaborator

It looks like the code in find_isinstance_check to refine optionals based on comparisons is explicitly gated on --strict-optional: https://github.com/python/mypy/blob/master/mypy/checker.py#L2865

Is there any reason to not just change that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants