Add __reversed__ Support, proper __repr__ for _collections_abc.KeysView

A The KeysView in _collections_abc.py should implement support for __reversed__ to match the expectation set forth in Dictionary view objects.

B The KeysView should also properly implement its own __repr__ to match how the built-in dict object reports its keys.

Below is the current implementation of KeysView:

class KeysView(MappingView, Set):

    __slots__ = ()

    @classmethod
    def _from_iterable(cls, it):
        return set(it)

    def __contains__(self, key):
        return key in self._mapping

    def __iter__(self):
        yield from self._mapping

My suggestions are

class KeysView(MappingView, Set):

    __slots__ = ()

    @classmethod
    def _from_iterable(cls, it):
        return set(it)

    def __repr__(self):  # B
        return f'{self.__class__.__name__}([{", ".join(repr(i) for i in self)}])'

    def __contains__(self, key):
        return key in self._mapping

    def __iter__(self):
        yield from self._mapping

    def __reversed__(self):  # A
        yield from reversed(self._mapping)

Currently, a class inheriting UserDict will not support key reversal, as shown below

from collections import UserDict


class CustomDict(UserDict):
    pass

>>> d = CustomDict()
>>> d['a'] = 'b'
>>> reversed(d.keys())
TypeError: 'KeysView' object is not reversible

This is in direct conflict with what is reported as supported in Dictionary View Objects:

reversed(dictview)

Return a reverse iterator over the keys, values or items of the dictionary. The view will be iterated in reverse order of the insertion.

Changed in version 3.8: Dictionary views are now reversible.

Also, per point B, the __repr__ for the default KeysView implementation incorrectly returns the repr of the _mapping object and not only the keys.

Using the above custom class, you will get the following for .keys()

>>> d = CustomDict({v: i for i, v in enumerate('spam')})
>>> d.keys()
KeysView({'s': 0, 'p': 1, 'a': 2, 'm': 3})

The KeysView object should only return the actual keys, like how dict works:

>>> d = {v: i for i, v in enumerate('spam')}
>>> d.keys()
dict_keys(['s', 'p', 'a', 'm'])

Ad B
Most often, __repr__ return an evaluable string. As KeysView subclasses MappingView, and uses its __init__, the mapping is required, not just the keys.

Ad A
The documentation states a dictview is reversible, not a MappingView. The former is ordered (so reversed makes sense), the latter makes no such promise (and then reversed is not sensible). E.g., reversed(set()) gives the same error.

The misconception here is that abstract classes should do everything that the related concrete classes do. This is usually not true. The abcs can be minimal usable subsets of the capabilities of the concrete classes. It is incorrect to say things like “to match the expectation set forth in Dictionary view objects”. The abc sets expectations, not the other way around.

A more critical problem is that it is not okay to add capabilities to existing abcs. Their apis are fixed at birth. This limitation is covered in the maintenance notes.

A last problem is that the view abcs are intended for general use. Not all mappings are ordered or easily reversible (for example lisplike association lists).

4 Likes

That makes sense.

Looking more into it, the OrderedDict handles this by subclassing the _collections.abc.KeysView and then implementing its own __reversed__ #L69. Something like that would probably be the correct approach for UserDict, if that’s something we’d want to support.

I didn’t even notice UserDict doesn’t support __reversed__ until now, as well, which I would expect it would.

1 Like

Would implementing the _OrderedDictKeysView, _OrderedDictItemsView, and _OrderedDictValuesView views for UserDict, like shown below, cause issues with the abc?

class _UserDictKeysView(_collections_abc.KeysView):

    def __repr__(self):
        _cls = self.__class__.__name__
        return f'{_cls}_keys([{", ".join(repr(i) for i in self.data)}])'

    def __reversed__(self):
        yield from reversed(self.data)

class _UserDictItemsView(_collections_abc.ItemsView):

    def __repr__(self):
        _cls = self.__class__.__name__
        return f'{_cls}_items([{", ".join(str(i) for i in self.data.items())}])'

    def __reversed__(self):
        for key in reversed(self.data):
            yield (key, self.data[key])

class _UserDictValuesView(_collections_abc.ValuesView):

    def __repr__(self):
        _cls = self.__class__.__name__
        return f'{_cls}_values([{", ".join(repr(i) for i in self.data.values())}])'

    def __reversed__(self):
        for key in reversed(self.data):
            yield self.data[key]

And then in UserDict add

class UserDict(_collections_abc.MutableMapping):

    ...

    def keys(self):
        "D.keys() -> a set-like object providing a view on D's keys"
        return _UserDictKeysView(self)

    def items(self):
        "D.items() -> a set-like object providing a view on D's items"
        return _UserDictItemsView(self)

    def values(self):
        "D.values() -> an object providing a view on D's values"
        return _UserDictValuesView(self)

    def __reversed__(self):
        yield from reversed(self.data)  # just for funsies

Why do you need to use a UserDict at first place? Why not use a dict or a dict subclass?

UserDict was introduced when it was impossible to subclass dict, and now it is kept for backward compatibility.

Inheriting from built-ins can cause unexpected oddities.

For instance, in Chapter 14: Inheritance: For Better or for Worse subsection “Subclassing Built-In Types is Tricky” in the 2nd Edition of Fluent Python[0], Luciano shows an example of the issue in Example 14-1, reproduced below.

# Example 14-1. Our __setitem__ override is ignored by the __init__ and __update__ methods of the built-in dict

>>> class DoppelDict(dict):
...		def __setitem__(self, key, value):
...			super().__setitem__(key, [value] * 2)
...
>>> dd = DoppelDict(one=1)
>>> dd
{'one': 1}
>>> dd['two'] = 2
>>> dd
{'one': 1, 'two': [2, 2]}
>> dd.update(three=3)
>> dd
{'three': 3, 'one': 1, 'two': [2, 2]}

You can compare this to a subclassed UserDict, which behaves more as you would expect:

>>> from collections import UserDict
>>> class DoppelUserDict(UserDict):
...     def __setitem__(self, key, value):
...         super().__setitem__(key, [value] * 2)
...
>>> dud = DoppelUserDict(one=1)
>>> dud
{'one': [1, 1]}
>>> dud['two'] = 2
>>> dud
{'one': [1, 1], 'two': [2, 2]}
>>> dud.update(three=3)
>>> dud
{'one': [1, 1], 'two': [2, 2], 'three': [3, 3]}

The section continues on to show more examples of rough-edges caused by inheriting from built-in types.

[0] Ramalho, Luciano. Fluent Python: 2nd Edition. “O’Reilly Media, Inc.,” 31 Mar. 2022.

EDITS: Typos, code cleanups.

4 Likes

I personally still actively use UserDict as a proxy to an existing dict that needs certain custom behaviors. That the data attribute can be easily modified to point to an existing data source is something a dict subclass can’t do.

2 Likes

I won’t consider this broken. Having __setattr__ change the incoming value to something entirely different is against the semantic contract of mutable mappings. There are indeed APIs where things don’t duck-type and the exact class matters, the behavior of dicts and other collections.abc.MutableMappings differ (like json serialization, and eval and execglobals argument). Emulating built-in dict without nominally inheriting from it may occasionally have its uses, but it’s not an excuse to introduce completely different semantics from dict / MutableMapping.