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

No type narrowing for in on dict.keys() #13360

Closed
matthewhughes934 opened this issue Aug 8, 2022 · 1 comment
Closed

No type narrowing for in on dict.keys() #13360

matthewhughes934 opened this issue Aug 8, 2022 · 1 comment
Labels
feature topic-type-narrowing Conditional type narrowing / binder

Comments

@matthewhughes934
Copy link
Contributor

Bug Report

A an if statement like if x in some_dict.keys() doesn't narrow the type of x to the type of the key of some_dict.
This is in contrast with if x in some_dict which does correctly narrow.

(I had a search and didn't find a similar existing discussion, apologies if this isn't the case)

To Reproduce

Run mypy over the following:

from collections.abc import KeysView


def get_via_keys(key: str | None, data: dict[str, str]) -> str:
    if key in data.keys():
        # error: Incompatible return value type (got "Optional[str]", expected "str")
        return key
    return "value"


def get_via_keys_explicit_typing(key: str | None, data: dict[str, str]) -> str:
    keys: KeysView[str] = data.keys()
    if key in keys:
        # error: Incompatible return value type (got "Optional[str]", expected "str")
        return key
    return "value"


def get_check_dict_membership(key: str | None, data: dict[str, str]) -> str:
    if key in data:
        # OK
        return key
    return "value"

Expected Behavior

mypy correctly narrows the type in each of the if statements and the script passes.

Actual Behavior

mypy reports failures as:

script.py:7: error: Incompatible return value type (got "Optional[str]", expected "str")
script.py:15: error: Incompatible return value type (got "Optional[str]", expected "str")
Found 2 errors in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: 0.971 and mypy 0.980+dev.e69bd9a7270daac8db409e8d08400d9d32367c32 (compiled: no) (current master)
  • Mypy command-line flags: mypy <name-of-file-above>
  • Mypy configuration options from mypy.ini (and other config files): the above was run from the root of this project (so whatever's configured there)
  • Python version used: Python 3.10.5
  • Operating system and version: Arch Linux (kernel 5.18.15)

The following allows get_via_keys above to pass under mypy

diff --git a/mypy/checker.py b/mypy/checker.py
index e64cea7b4..852fe8fab 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -6285,6 +6285,7 @@ def builtin_item_type(tp: Type) -> Optional[Type]:
             "builtins.dict",
             "builtins.set",
             "builtins.frozenset",
+            "_collections_abc.dict_keys",
         ]:
             if not tp.args:
                 # TODO: fix tuple in lib-stub/builtins.pyi (it should be generic).

Though I'm not sure how appropriate it would be given the note in the docs for that function:

Note: this is only OK for built-in containers, where we know the behavior of __contains__.

Also, dict_keys is undocumented and a quick git grep --word-regexp 'dict_keys' didn't show much usage for it outside of typeshed.

@matthewhughes934 matthewhughes934 added the bug mypy got something wrong label Aug 8, 2022
@matthewhughes934 matthewhughes934 changed the title No type narrowing for dict.keys() No type narrowing for ìn`on dict.keys() Aug 8, 2022
@matthewhughes934 matthewhughes934 changed the title No type narrowing for ìn`on dict.keys() No type narrowing for inon dict.keys() Aug 8, 2022
@matthewhughes934 matthewhughes934 changed the title No type narrowing for inon dict.keys() No type narrowing for in on dict.keys() Aug 8, 2022
@sobolevn
Copy link
Member

sobolevn commented Aug 9, 2022

Note: this is only OK for built-in containers, where we know the behavior of contains.

We know that dict.keys() is safe! Please, feel free to send a PR.

@AlexWaygood AlexWaygood added feature topic-type-narrowing Conditional type narrowing / binder and removed bug mypy got something wrong labels Aug 9, 2022
matthewhughes934 added a commit to matthewhughes934/mypy that referenced this issue Aug 9, 2022
This is to bring the following cases into alignment:

    key: str | None
    d: dict[str, str]

    if key in d:
        # type of 'key' is inferred to be 'str'
        ...

    if key in d.keys():
        # type of 'key' should be inferred to be 'str'
        ...

Before this change only the first `if` would narrow the type of `key`.

GH: issue python#13360
matthewhughes934 added a commit to matthewhughes934/mypy that referenced this issue Aug 16, 2022
Also for containership checks on `typing.KeysView.` This is to bring the
following cases into alignment:

    from typing import KeysView

    key: str | None
    d: dict[str, int]
    kv: KeysView[str]

    if key in d:
        # type of 'key' is inferred to be 'str'
        ...

    if key in d.keys():
        # type of 'key' should be inferred to be 'str'
        ...

    if key in kv:
        # type of 'key' should be inferred to be 'str'
        ...

Before this change only the first `if` would narrow the type of `key`.

I've just added a test under `test-data/unit/pythoneval.test` as
`test-data/unit/fixtures/dict.pyi` doesn't include `dict.keys`, and
adding it requires importing `dict_keys` from `_collections_abc` in
those stubs, which then requires adding `_collections_abc.pyi` stubs,
which would have to be complete since e.g.
`testGenericAliasCollectionsABCReveal` expects most of the types in
those stubs to be defined (this is the same approach as
7678f28).

GH: issue python#13360
matthewhughes934 added a commit to matthewhughes934/mypy that referenced this issue Aug 23, 2022
Also for containership checks on `typing.KeysView.` This is to bring the
following cases into alignment:

    from typing import KeysView

    key: str | None
    d: dict[str, int]
    kv: KeysView[str]

    if key in d:
        # type of 'key' is inferred to be 'str'
        ...

    if key in d.keys():
        # type of 'key' should be inferred to be 'str'
        ...

    if key in kv:
        # type of 'key' should be inferred to be 'str'
        ...

Before this change only the first `if` would narrow the type of `key`.

I've just added a test under `test-data/unit/pythoneval.test` as
`test-data/unit/fixtures/dict.pyi` doesn't include `dict.keys`, and
adding it requires importing `dict_keys` from `_collections_abc` in
those stubs, which then requires adding `_collections_abc.pyi` stubs,
which would have to be complete since e.g.
`testGenericAliasCollectionsABCReveal` expects most of the types in
those stubs to be defined (this is the same approach as
7678f28).

GH: issue python#13360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature topic-type-narrowing Conditional type narrowing / binder
Projects
None yet
Development

No branches or pull requests

3 participants