KeysView is now incompatible dict.keys() return type

[mattdavis90]

Bug Report

When sub-classing a Dict and overloading the keys method the following error occurs when updating from mypy v0.921 to v0.930.

Return type "KeysView[_K]" of "keys" incompatible with return type "dict_keys[_K, _V]" in supertype "dict"

To Reproduce

With v0.291 this code is perfectly valid and passes validation.

from typing import TypeVar, Dict
from collections.abc import KeysView

_K = TypeVar("_K")
_V = TypeVar("_V")

class A(Dict[_K, _V]):
    def __init__(self):
        self._dict: Dict[_K, _V] = {}

    def keys(self) -> KeysView[_K]:
        return self._dict.keys()

https://mypy-play.net/?mypy=0.921&python=3.10&gist=7159c56ca6010a5aace637383a18b8b1

Post update to v0.930 the above code fails validation and requires updating to the following.

from typing import TypeVar, Dict
from _collections_abc import dict_keys

_K = TypeVar("_K")
_V = TypeVar("_V")

class A(Dict[_K, _V]):
    def __init__(self):
        self._dict: Dict[_K, _V] = {}

    def keys(self) -> dict_keys[_K, _V]:
        return self._dict.keys()

https://mypy-play.net/?mypy=latest&python=3.10&gist=65a7626023630e18d178acd0bd75ec61

Expected Behavior

The dict_keys class extends the KeysView class so I would have expected that the code could have remained the same and mypy would walk the hierarchy to determine that the code is still valid. For instance, this shows that the KeysView should work (IMO) on v0.930

from collections.abc import KeysView
reveal_type({}.keys())  # Revealed type is "_collections_abc.dict_keys[<nothing>, <nothing>]"
print(isinstance({}.keys(), KeysView))  # True

Actual Behavior

The class must be rewritten to use the new dict_keys return type which is not available in a "public" package.

Your Environment

• Mypy version used: v0.921 and v0.930
• Mypy command-line flags: N/A
• Mypy configuration options from mypy.ini (and other config files): N/A
• Python version used: 3.6, 3.7, 3.8, 3.9, 3.10
• Operating system and version: Playground

[hauntsaninja]

Yeah, unfortunate. This is a consequence of #6039 , the difference comes down to whether (on Python 3.10) A().keys().mapping works or not.
Maybe we could conditionally add the mapping attribute to the views classes in typing?

[mattdavis90]

@hauntsaninja I wasn't 100% sure if that question mark was aimed at me or the mypy/typeshed maintainers so I gave it a couple of days before having a go at a patch (#6888).

[Akuli]

I think it's getting a bit unclear what exactly do classes benefiting from this look like. I'm looking at this:

class A(Dict[_K, _V]):
    def __init__(self):
        self._dict: Dict[_K, _V] = {}

    def keys(self) -> KeysView[_K]:
        return self._dict.keys()

This is broken by design, because you have two distinct dicts, self and self._dict, that can contain different data. To make sure that everything accesses self._dict, you could override every dict method that exists, but even that wouldn't fix C code that modifies dicts and presumably isn't programmed to call methods defined in Python.

Is there a real-world example of a dict subclass that isn't broken and where this problem occurs?

@mattdavis90 and @zzzeek, do you have any not-broken examples of dict subclasses that override keys() to do something else than return super().keys() unchanged?

[zzzeek]

@Akuli I have a few that do "values()" differently, although if they are considered to be "broken" is up for debate - they all can certainly be implemented as MutableMapping objects instead and maybe we should just be doing that, and in fact I probably will seek to do this to work around the issues we're having.

The guidance I was looking for at python/typing#1033 may have been for someone to tell me "no, you really shouldn't be doing that, use a MutableMapping as your base instead". I think some docs somewhere to this effect should be giving us all the heads up that subclassing typing.Dict and overriding keys() or values() is officially an anti-pattern. Here's one that I've somewhat paraphrased from SQLAlchemy, I can find more if interested:

class LRUCache(dict):
    """Dictionary with 'squishy' removal of least
    recently used items.

    """

    def __init__(self, capacity=100, threshold=0.5):
        self.capacity = capacity
        self.threshold = threshold
        self._counter = 0
        self._mutex = threading.Lock()

    def _inc_counter(self):
        self._counter += 1
        return self._counter

    def get(self, key, default=None):
        item = dict.get(self, key, default)
        if item is not default:
            item[2] = self._inc_counter()
            return item[1]
        else:
            return default

    def __getitem__(self, key):
        item = dict.__getitem__(self, key)
        item[2] = self._inc_counter()
        return item[1]

    def values(self):
        return [i[1] for i in dict.values(self)]

    def setdefault(self, key, value):
        if key in self:
            return self[key]
        else:
            self[key] = value
            return value

    def __setitem__(self, key, value):
        item = dict.get(self, key)
        if item is None:
            item = [key, value, self._inc_counter()]
            dict.__setitem__(self, key, item)
        else:
            item[1] = value
        self._manage_size()

    @property
    def size_threshold(self):
        return self.capacity + self.capacity * self.threshold

    def _manage_size(self):
        if not self._mutex.acquire(False):
            return
        try:
            while len(self) > self.capacity + self.capacity * self.threshold:
                if size_alert:
                    size_alert = False
                by_counter = sorted(
                    dict.values(self), key=operator.itemgetter(2), reverse=True
                )
                for item in by_counter[self.capacity :]:
                    try:
                        del self[item[0]]
                    except KeyError:
                        # deleted elsewhere; skip
                        continue
        finally:
            self._mutex.release()

[JelleZijlstra]

Thanks @zzzeek! It's helpful to have concrete examples to look at.

Your example is incompatible with dict.values anyway, since you're returning a list, and there are several non-overridden methods that won't behave right (e.g., pop, items).

I don't think we should do something as heavy as adding a new class to typing-extensions, since this is a rare use case and many uses aren't fully type-safe anyway. Users who run into this can just either # type: ignore or inherit from Mapping/MutableMapping instead of dict.

[zzzeek]

oh the list part, that's wrong I was using a ValuesView for that when i was getting it to work, that's not the main "broken" part of it.

I'm going to change our classes to use MutableMapping.

Question though looking in stdlib at for example WeakValueDictionary the values method is returning an iterator, not ValuesView. is that wrong?

I'm not able to find in typing / typeshed / cpython source where the actual annotations for Mapping / Dict / MutableMapping etc. are, the packages all seem to point to each other but I don't see the annotations anywhere, how do I find that ?

[AlexWaygood]

I'm not able to find in typing / typeshed / cpython source where the actual annotations for Mapping / Dict / MutableMapping etc. are, the packages all seem to point to each other but I don't see the annotations anywhere, how do I find that ?

• Mapping is here.
• MutableMapping is here.
• dict is here.

[Akuli]

WeakValueDictionary the values method is returning an iterator

This is more of a CPython problem than a typing problem. It can cause type checkers to accept code like weak_value_dictionary.keys() | {1, 2}, even though it will fail at runtime. Not great, but also not something that we can do much about, unless we make yet another CPython bug report to be rejected.

[zzzeek]

OK so ....cypthon stblib itself is not correctly typed and there's some trend of them rejecting bug reports to that effect?

[JelleZijlstra]

Classes like WeakValuesDictionary long predate typing. They don't obey the Liskov Substitution Principle, but that's unlikely to be fixed because it causes little trouble in practice and would be difficult to fix without breaking backward compatibility.

[Akuli]

Indeed, CPython's stdlib is not typed, and hopefully won't be typed any time soon. See also #5444.

My general impression is that CPython developers don't like to change things just because they cause problems in typing. But that's mostly just me ranting, feel free to ignore that part :)

[AlexWaygood]

Question though looking in stdlib at for example WeakValueDictionary the values method is returning an iterator, not ValuesView. is that wrong?

If you look at the typeshed annotation for WeakValueDictionary, it has # type: ignore[override], because otherwise mypy would complain about the LSP violation. And, to be clear -- you should also see that as a viable option! mypy is correct to complain about LSP being violated -- but it's not always the end of the planet. Violating LSP and telling mypy to shut up is sometimes the best way to go :)

[zzzeek]

OK, I wasnt really getting how to use the ignore[code] part because those codes dont come out in the mypy messages, but your comment led me to read the docs again to learn about the --show-error-codes option, that should be helpful thanks.

nevertheless, all the looking at my LRUCache shows me that this is clearly a MutableMapping case and there's no reason for it to subclass Dict.

[Akuli]

the --show-error-codes option

You can also set show_error_codes = True in the [mypy] section of your mypy.ini.

[KotlinIsland]

@mattdavis90

I would have expected that the code could have remained the same and mypy would walk the hierarchy to determine that the code is still valid

I wouldn't, the return type of the base class's implementation has been updated to dict_keys, so the return type in this class must be the same or a subtype.

dict_keys is a subtype of KeysView and is more specialized, to say that your subtype will return KeysView would make it not respect LSP and would lead to type errors if the implementation did indeed return a KeysView instead of a dict_keys:

class A(Dict[_K, _V]):
    def __init__(self):
        self._dict: Dict[_K, _V] = {}

    def keys(self) -> KeysView[_K]:
        return self._dict.keys()

class B(A):
    def keys(self) -> KeysView[_K]:
        return KeysView({})

d: dict[object, object] = B()
d.keys().mapping # AttributeError: 'KeysView' object has no attribute 'mapping'. Did you mean: '_mapping'?

mypy would walk the hierarchy

I don't understand what you mean by this, following that logic why not just annotate it as object, and expect mypy to 'walk the
hierarchy' and determine that it is actually returning a dict_keys?

class A(Dict[_K, _V]):
    def __init__(self):
        self._dict: Dict[_K, _V] = {}

    def keys(self) -> object:
        return self._dict.keys()

I believe this is an invalid issue and should be closed.

[KotlinIsland]

@hauntsaninja

Maybe we could conditionally add the mapping attribute to the views classes in typing?

Why would you add it to the KeysView/ValuesView stubs when that attribute doesn't actually exist on those types?

[hauntsaninja]

You're missing discussion that happened in another PR.

This is my summary of the situation here: #6888 (comment)