This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Akuli, AlexWaygood, Dennis Sweeney, JelleZijlstra, kj, kumaraditya, methane, rhettinger, serhiy.storchaka, sobolevn
Priority: normal Keywords: 3.10regression, patch

Created on 2022-01-16 13:55 by AlexWaygood, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 30629 closed kumaraditya, 2022-01-17 09:14
PR 30630 closed kumaraditya, 2022-01-17 09:51
Messages (21)
msg410695 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-01-16 13:55
Issue40890 added a new `.mapping` attribute to dict_keys, dict_values and dict_items in 3.10. This addition is great for introspection. However, it has inadvertently caused some unexpected problems for type-checkers.

Prior to Issue40890, the typeshed stub for the three `dict` methods was something like this:

```
from typing import MutableMapping, KeysView, ItemsView, ValuesView, TypeVar

_KT = TypeVar("_KT")
_VT = TypeVar("_VT")

class dict(MutableMapping[_KT, _VT]):
    def keys(self) -> KeysView[_KT]: ...
    def values(self) -> ValuesView[_VT]: ...
    def items(self) -> ItemsView[_KT, _VT]: ...
```

In other words, typeshed did not acknowledge the existence of a "dict_keys" class at all. Instead, it viewed the precise class as an implementation detail, and merely stated in the stub that `dict.keys()` would return some class that implemented the `KeysView` interface.

After Issue40890, however, it was clear that this approach would no longer suffice, as mypy (and other type-checkers) would yield false-positive errors for the following code:

```
m = dict().keys().mapping
```

Following several PRs, the typeshed stub for these `dict` methods now looks something like this:

```
# _collections_abc.pyi

import sys
from types import MappingProxyType
from typing import Generic, KeysView, ValuesView, ItemsView, TypeVar, final

_KT_co = TypeVar("_KT_co", covariant=True)
_VT_co = TypeVar("_VT_co", covariant=True)

@final
class dict_keys(KeysView[_KT_co], Generic[_KT_co, _VT_co]):
    if sys.version_info >= (3, 10):
        mapping: MappingProxyType[_KT_co, _VT_co]

@final
class dict_values(ValuesView[_VT_co], Generic[_KT_co, _VT_co]):
    if sys.version_info >= (3, 10):
        mapping: MappingProxyType[_KT_co, _VT_co]

@final
class dict_items(ItemsView[_KT_co, _VT_co], Generic[_KT_co, _VT_co]):    
    if sys.version_info >= (3, 10):
        mapping: MappingProxyType[_KT_co, _VT_co]

# builtins.pyi

from _collections_abc import dict_keys, dict_views, dict_items
from typing import MutableMapping, TypeVar

_KT = TypeVar("_KT")
_VT = TypeVar("_VT")

class dict(MutableMapping[_KT, _VT]):
    def keys(self) -> dict_keys[KT, VT]: ...
    def values(self) -> dict_values[_KT, _VT]: ...
    def items(self) -> dict_items[_KT, _VT]: ...
```

The alteration to the typeshed stub means that mypy will no longer give false-positive errors for code in which a user attempts to access the `mapping` attribute. However, it has serious downsides. Users wanting to create typed subclasses of `dict` have found that they are no longer able to annotate `.keys()`, `.values()` and `.items()` as returning `KeysView`, `ValuesView` and `ItemsView`, as mypy now flags this as an incompatible override. Instead, they now have to import `dict_keys`, `dict_values` and `dict_items` from `_collections_abc`, a private module. Moreover, they are unable to parameterise these classes at runtime.

In other words, you used to be able to do this:

```
from typing import KeysView, TypeVar

K = TypeVar("K")
V = TypeVar("V")

class DictSubclass(dict[K, V]):
    def keys(self) -> KeysView[K]:
        return super().keys()
```

But now, you have to do this:

```
from _collections_abc import dict_keys
from typing import TypeVar

K = TypeVar("K")
V = TypeVar("V")

class DictSubclass(dict[K, V]):
    def keys(self) -> "dict_keys[K, V]":
        return super().keys()
```

References:
* PR where `.mapping` attribute was added to the typeshed stubs: https://github.com/python/typeshed/pull/6039
* typeshed issue where this was recently raised: https://github.com/python/typeshed/issues/6837
* typeshed PR where this was further discussed: https://github.com/python/typeshed/pull/6888
msg410696 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-01-16 13:58
As a fix, I propose that two changes be made to `dict_keys`, `dict_values` and `dict_items`:

* They should be officially exposed in the `types` module.
* `__class_getitem__` should be added to the classes.

These two changes would mean that users would be able to do the following:

```
from types import dict_keys
from typing import TypeVar

K = TypeVar("K")
V = TypeVar("V")

class DictSubclass(dict[K, V]):
    def keys(self) -> dict_keys[K, V]:
        return super().keys()
```
msg410699 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2022-01-16 16:21
> I propose that two changes be made to `dict_keys`, `dict_values`
> and `dict_items`:
>
> * They should be officially exposed in the `types` module.
> * `__class_getitem__` should be added to the classes.

+1
msg410703 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2022-01-16 16:53
Okay, PR welcome.

(Though since this can only work for 3.11+,
maybe typeshed could also be adjusted?
For that you’d have to file a bug there.)
msg410752 - (view) Author: Kumar Aditya (kumaraditya) * (Python triager) Date: 2022-01-17 09:41
I would like to work on this.
msg410753 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-01-17 10:13
Thanks, Kumar — I appreciate it!

> (Though since this can only work for 3.11+, maybe typeshed could also be adjusted? For that you’d have to file a bug there.)

Yes — it's unfortunate, but I have some ideas for hacky workarounds we can implement in typeshed for 3.10 :) and, as you say — that's a problem to be dealt with on the typeshed issue tracker rather than here.
msg410754 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-01-17 10:14
Do these changes warrant an entry in "What's New in 3.11"?
msg410822 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2022-01-17 20:45
Probably.

On Mon, Jan 17, 2022 at 02:14 Alex Waygood <report@bugs.python.org> wrote:

>
> Alex Waygood <Alex.Waygood@Gmail.com> added the comment:
>
> Do these changes warrant an entry in "What's New in 3.11"?
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue46399>
> _______________________________________
>
-- 
--Guido (mobile)
msg410921 - (view) Author: Jelle Zijlstra (JelleZijlstra) * (Python committer) Date: 2022-01-19 02:29
The types of `.keys()`, `.items()`, and `.values()` on `collections.OrderedDict` are distinct from those for dict, and they are also not exposed anywhere. Should we put them in a public, documented place too for consistency?

>>> import collections
>>> od = collections.OrderedDict()
>>> type(od.keys())
<class 'odict_keys'>
msg410923 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2022-01-19 04:53
I am not happy about exposing every internal types. I prefer duck typing.

Like OrderedDict, not all dict subtypes uses `dict_keys`, `dict_views`, and `dict_items`.
If typeshed annotate dict.keys() returns `dict_keys`, "incompatible override" cano not be avoided.

I prefer:

* Keep status-quo: keys().mapping cause false positive and user need to suppress. This is not a big problem because `.mapping` is very rarely used.
* Or add `.mapping` to `KeysView`, `ValuesView`, and `ItemsView`. Force every dict subclasses to implement it.
msg410928 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-01-19 08:59
I agree with Inada that not every internal type should be exposed, but I would make an exception for the dict views classes due to the fact that dict subclasses are much more common than subclasses of other mappings, such as OrderedDict. I don't think it's *particularly* important to expose the OrderedDict views classes in the same way.

Adding the "mapping" attribute to KeysView/ValuesView/ItemsView seems like it could be quite disruptive — there may be a lot of third-party users with classes that inherit from those, who'd need to make changes to their code. From a typing perspective, it would also mean that KeysView and ValuesView would have to be parameterised with two TypeVars (key-type and value-type), whereas now they both only take one (KeysView is parameterised with the key-type, ValuesView with the value-type).
msg410929 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2022-01-19 09:48
> I agree with Inada that not every internal type should be exposed, but I would make an exception for the dict views classes due to the fact that dict subclasses are much more common than subclasses of other mappings, such as OrderedDict. I don't think it's *particularly* important to expose the OrderedDict views classes in the same way.

I am afraid that you misread me. I used OrderedDict as one example of dict subclass. I didn't mean dict_(keys|items|values) shouldn't exposed because of I don't want to expose odict_(keys|items|values).

Anyway, OrderedDict was not good choise to explain my thought because its builtin type and defined in typeshed. Instead, I use sortedcontainers.SortedDict as example.

See https://github.com/grantjenks/python-sortedcontainers/blob/dff7ef79a21b3f3ceb6a19868f302f0a680aa243/sortedcontainers/sorteddict.py#L43

It is a dict subclass. It's `keys()` method returns `SortedKeysView`.
`SortedKeysView` is subclass of `collections.abc.KeysView`. But it is not subclass of `dict_keys`.
If `dict.keys()` in typeshed defines it returns `dict_keys`, doesn't mypy flag it as an "incompatible override"?

So I propose that typeshed defines that dict.keys() returns KeysView, not dict_keys.

Although subclass of dict is very common, it is very rare that:

* Override `keys()`, and
* Returns `super().keys()`, instead of KeysView (or list), and
* `.keys().mapping` is accessed.

It is very minor inconvinience that user need to ignore false positive for this very specific cases.
Or do you think this case is much more common than classes like SortedDict?

Note that dict_(keys|items|values) is implementation detail and subclassing it doesn't make sense.

Another option is adding more ABC or Protocol that defines `.mapping` attribute.
SortedKeysView can inherit it and implement `.mapping`.
msg410930 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2022-01-19 10:10
In other words,

a. If `.keys()` in all dict subclasses must return subclass of `dict_keys`: `dict.keys() -> dict_keys`.
b. If `.keys().mapping` must be accessible for all dict subclasses: Add `.mapping` to `KeysView`.
c. If `.keys().mapping` is optional for dict subclasses: typeshed can't add `.mapping` to anywhere, AFAIK.
msg410933 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-01-19 10:23
I share concerns of Inada-san. I also think that keeping a status quo (ignoring the mapping attribute in typing) is the lesser evil. I am not sure that exposing this attribute was a good idea. We do not expose attributes list and index for list iterators.
msg410955 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2022-01-19 14:59
Given all this discussion I defer to Serhiy and Inada-San.

On Wed, Jan 19, 2022 at 02:23 Serhiy Storchaka <report@bugs.python.org>
wrote:

>
> Serhiy Storchaka <storchaka+cpython@gmail.com> added the comment:
>
> I share concerns of Inada-san. I also think that keeping a status quo
> (ignoring the mapping attribute in typing) is the lesser evil. I am not
> sure that exposing this attribute was a good idea. We do not expose
> attributes list and index for list iterators.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue46399>
> _______________________________________
>
-- 
--Guido (mobile)
msg410960 - (view) Author: Jelle Zijlstra (JelleZijlstra) * (Python committer) Date: 2022-01-19 15:38
I see the concerns about exposing too many implementation details.

But I'm also not sure how much this will really help static typing use cases. Alex's examples just call super().keys(), but if you do that, there's not much point in overriding keys() in the first place.

These classes don't allow subclassing or instantiation:

>>> t = type({}.items())
>>> class X(t):
...     pass
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: type 'dict_items' is not an acceptable base type
>>> t({})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'dict_items' instances

So I can't think of much useful, well-typed code that you could write if these classes were public. For a use case like SortedDict, you'd still need to write your own class implementing KeysView, and you'd get an error from the type checker because it's incompatible with dict_keys.
msg411020 - (view) Author: Akuli (Akuli) Date: 2022-01-20 12:16
> I also think that keeping a status quo (ignoring the mapping attribute in typing) is the lesser evil.

Can you clarify this? There are several things that typing could do, and I don't know which option you are referring to. I listed most of them below, and I'd like to know which exactly of them "is the lesser evil".

If we literally ignore the attribute, any usage of `.mapping` will be an error, which basically makes the whole `.mapping` feature useless for statically typed code. It also wouldn't appear in IDE autocompletions.

If we add it to `KeysView` and `ValuesView`, library authors will end up using `.mapping` with arguments annotated as `Mapping` or `MutableMapping`, not realizing it is purely a dict thing, not required from an arbitrary mapping object.

If we keep `.mapping` in dict but not anywhere else, as described already, it becomes difficult to override .keys() and .values() in a dict subclass. You can't just return a KeysView or a ValuesView. If that was allowed, how should people annotate code that uses `.mapping`? You can't annotate with `dict`, because that also allows subclasses of dict, which might not have a `.mapping` attribute.

Yet another option would be to expose `dict_keys` and `dict_values` somewhere where they don't actually exist at runtime. This leads to code like this:


from typing import Any, TYPE_CHECKING
if TYPE_CHECKING:
    # A lie for type checkers to work.
    from something_that_doesnt_exist_at_runtime import dict_keys, dict_values
else:
    # Runtime doesn't check type annotations anyway.
    dict_keys = Any
    dict_values = Any


While this works, it isn't very pretty.
msg411065 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2022-01-21 01:08
> If we literally ignore the attribute, any usage of `.mapping` will be an error, which basically makes the whole `.mapping` feature useless for statically typed code. It also wouldn't appear in IDE autocompletions.

`.mapping` is not exist between Python 3.0~3.9. And it is not feature that is long awaited by many users.

See https://bugs.python.org/issue40890#msg370841
Raymond said:

  Traditionally, we do expose wrapped objects:  property() exposes fget,  partial() exposes func, bound methods expose __func__, ChainMap() exposes maps, etc.
  Exposing this attribute would help with introspection, making it possible to write efficient functions that operate on dict views.

Type hints is very useful for application code, especially when it is large.
But introspection is very rarely used in such typed code bases. I don't think `.mapping` is useful for many users, like `.fget` of the property.
So adding `# type: ignore` in such lines is the "lesser evil".


> If we add it to `KeysView` and `ValuesView`, library authors will end up using `.mapping` with arguments annotated as `Mapping` or `MutableMapping`, not realizing it is purely a dict thing, not required from an arbitrary mapping object.

It doesn't make sense at all, IMO.
If we really need `.mapping` in typeshed, we should add it to `KeysViewWithMapping`.
So mapping classes that don't inherit dict shouldn't be forced to implement `.mapping`.


> If we keep `.mapping` in dict but not anywhere else, as described already, it becomes difficult to override .keys() and .values() in a dict subclass. You can't just return a KeysView or a ValuesView. If that was allowed, how should people annotate code that uses `.mapping`? You can't annotate with `dict`, because that also allows subclasses of dict, which might not have a `.mapping` attribute.

`# type: ignore`.


> Yet another option would be to expose `dict_keys` and `dict_values` somewhere where they don't actually exist at runtime. This leads to code like this:
>
> from typing import Any, TYPE_CHECKING
> if TYPE_CHECKING:
>     # A lie for type checkers to work.
>     from something_that_doesnt_exist_at_runtime import dict_keys, dict_values
> else:
>     # Runtime doesn't check type annotations anyway.
>     dict_keys = Any
>     dict_values = Any
>
> While this works, it isn't very pretty.

What problem this problem solve? `SortedDict.keys()` can not return `dict_keys`.
As far as I think, your motivation is making dict subclass happy with type checkers.
But this option doesn't make dict subclass happy at all.
msg411098 - (view) Author: Akuli (Akuli) Date: 2022-01-21 08:55
I now agree that `# type: ignore` in dict subclasses makes sense the most, and exposing the views doesn't solve practical problems.

We talked more with the people who brought this up in typing. Turns out that the correct solution to their problems was to just inherit from MutableMapping instead of inheriting from dict. If you really need to inherit from dict and make .keys() do something else than it does by default, you're most likely trying to override all dict methods that do something with the keys, and you should just inherit from MutableMapping instead.
msg411115 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-01-21 11:59
I have also been persuaded that my suggested solution is not the way to go. Thanks everybody for the useful discussion.
msg411239 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-01-22 09:19
Thanks for the PRs, Kumar — I appreciate you putting in the time. Sorry for the wasted effort.
History
Date User Action Args
2022-04-11 14:59:54adminsetgithub: 90557
2022-01-22 09:19:51AlexWaygoodsetstatus: open -> closed
resolution: not a bug
messages: + msg411239

stage: patch review -> resolved
2022-01-21 11:59:29AlexWaygoodsetmessages: + msg411115
2022-01-21 08:55:19Akulisetmessages: + msg411098
2022-01-21 01:29:27gvanrossumsetnosy: - gvanrossum
2022-01-21 01:08:45methanesetmessages: + msg411065
2022-01-20 12:16:01Akulisetnosy: + Akuli
messages: + msg411020
2022-01-19 15:38:45JelleZijlstrasetmessages: + msg410960
2022-01-19 14:59:16gvanrossumsetmessages: + msg410955
2022-01-19 10:23:29serhiy.storchakasetmessages: + msg410933
2022-01-19 10:10:43methanesetmessages: + msg410930
2022-01-19 09:48:22methanesetmessages: + msg410929
2022-01-19 08:59:44AlexWaygoodsetmessages: + msg410928
2022-01-19 04:53:13methanesetnosy: + methane
messages: + msg410923
2022-01-19 02:29:23JelleZijlstrasetmessages: + msg410921
2022-01-17 20:45:09gvanrossumsetmessages: + msg410822
2022-01-17 10:14:06AlexWaygoodsetmessages: + msg410754
2022-01-17 10:13:18AlexWaygoodsetmessages: + msg410753
2022-01-17 09:51:15kumaradityasetpull_requests: + pull_request28833
2022-01-17 09:41:35kumaradityasetmessages: + msg410752
2022-01-17 09:14:25kumaradityasetkeywords: + patch
stage: patch review
pull_requests: + pull_request28832
2022-01-17 08:56:21kumaradityasetnosy: + kumaraditya
2022-01-16 16:53:17gvanrossumsetmessages: + msg410703
2022-01-16 16:21:56rhettingersetmessages: + msg410699
2022-01-16 13:58:59AlexWaygoodsetmessages: + msg410696
2022-01-16 13:55:19AlexWaygoodcreate