[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Alex Waygood]

Alex Waygood  added the comment:

Thanks for the PRs, Kumar — I appreciate you putting in the time. Sorry for the 
wasted effort.

--
resolution:  -> not a bug
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Alex Waygood]

Alex Waygood  added the comment:

I have also been persuaded that my suggested solution is not the way to go. 
Thanks everybody for the useful discussion.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Akuli]

Akuli  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Guido van Rossum]

Change by Guido van Rossum :


--
nosy:  -gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Inada Naoki]

Inada Naoki  added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Akuli]

Akuli  added the comment:

> 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.

--
nosy: +Akuli

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Jelle Zijlstra]

Jelle Zijlstra  added the comment:

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 "", line 1, in 
TypeError: type 'dict_items' is not an acceptable base type
>>> t({})
Traceback (most recent call last):
  File "", line 1, in 
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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Guido van Rossum]

Guido van Rossum  added the comment:

Given all this discussion I defer to Serhiy and Inada-San.

On Wed, Jan 19, 2022 at 02:23 Serhiy Storchaka 
wrote:

>
> Serhiy Storchaka  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 
> 
> ___
>
-- 
--Guido (mobile)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Serhiy Storchaka]

Serhiy Storchaka  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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Inada Naoki]

Inada Naoki  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Inada Naoki]

Inada Naoki  added the comment:

> 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`.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Alex Waygood]

Alex Waygood  added the comment:

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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Inada Naoki]

Inada Naoki  added the comment:

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.

--
nosy: +methane

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Jelle Zijlstra]

Jelle Zijlstra  added the comment:

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())


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Guido van Rossum]

Guido van Rossum  added the comment:

Probably.

On Mon, Jan 17, 2022 at 02:14 Alex Waygood  wrote:

>
> Alex Waygood  added the comment:
>
> Do these changes warrant an entry in "What's New in 3.11"?
>
> --
>
> ___
> Python tracker 
> 
> ___
>
-- 
--Guido (mobile)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Alex Waygood]

Alex Waygood  added the comment:

Do these changes warrant an entry in "What's New in 3.11"?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Alex Waygood]

Alex Waygood  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Kumar Aditya]

Change by Kumar Aditya :


--
pull_requests: +28833
pull_request: https://github.com/python/cpython/pull/30630

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Kumar Aditya]

Kumar Aditya  added the comment:

I would like to work on this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Kumar Aditya]

Change by Kumar Aditya :


--
keywords: +patch
pull_requests: +28832
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/30629

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Kumar Aditya]

Change by Kumar Aditya :


--
nosy: +kumaraditya303

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Guido van Rossum]

Guido van Rossum  added the comment:

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.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Raymond Hettinger]

Raymond Hettinger  added the comment:

> 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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Alex Waygood]

Alex Waygood  added the comment:

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()
```

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46399] Addition of `mapping` attribute to dict views classes has inadvertently broken type-checkers

[Alex Waygood]

New submission from Alex Waygood :

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

--
components: Library (Lib)
keywords: 3.10regression
messages: 410695
nosy: AlexWaygood, Dennis Sweeney, Jelle Zijlstra, gvanrossum, kj, rhettinger, 
serhiy.storchaka, sobolevn
priority: normal
severity: normal
status: open
title: Addition of `mapping` attribute to dict views classes has inadvertently 
broken type-checkers
type: behavior
versions: Python 3.10, Python 3.11

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com