Skip to content

Add __class_getitem__ to map builtin, PEP 585 #108761

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

Open
sobolevn opened this issue Sep 1, 2023 · 16 comments
Open

Add __class_getitem__ to map builtin, PEP 585 #108761

sobolevn opened this issue Sep 1, 2023 · 16 comments
Assignees
Labels
topic-typing type-feature A feature request or enhancement

Comments

@sobolevn
Copy link
Member

sobolevn commented Sep 1, 2023

Feature or enhancement

Right now it is impossible to subscribe map:

» ./python.exe
Python 3.13.0a0 (heads/main-dirty:79823c103b6, Aug 31 2023, 15:46:08) [Clang 14.0.3 (clang-1403.0.22.14.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> map[str]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: type 'map' is not subscriptable
>>> 

I propose adding __class_getitem__ support to map, because it makes sense in some cases.
However, https://peps.python.org/pep-0585/ does not say anything about map.

First of all, map is defined as Generic in typeshed:

class map(Iterator[_S], Generic[_S]):
    def __next__(self) -> _S: ...

Full def: https://github.com/python/typeshed/blob/9552ec7f72f3190eb529aec096783c624b43bd29/stdlib/builtins.pyi#L1473-L1514

So, any person that wants to, say, return map[str] right now has to use 'map[str]' (via '' or via from __future__ import annotations).

Second, while we can treat map[T] as just Iterator[T] in parameter types, we always want to as specific as possible in return types.

Third, it is a builtin, so we assume that it is widely used. And it actually is, even in CPython's source itself we can find usages that can be annotated as map[whatever]:

Lastly, it is a one-line change (+ tests) and many other builtins and stdlib objects support that already.

I cannot really think of any major cons.

Linked PRs

@sobolevn sobolevn added type-feature A feature request or enhancement topic-typing labels Sep 1, 2023
@sobolevn sobolevn self-assigned this Sep 1, 2023
sobolevn added a commit to sobolevn/cpython that referenced this issue Sep 1, 2023
@AlexWaygood
Copy link
Member

What about other builtin iterator classes, such as reversed and filter. Should support be added for those as well?

@sobolevn
Copy link
Member Author

sobolevn commented Sep 1, 2023

I don't know :)
This issue is just about map, I will try to make my research on them in separate issues.
I don't want to bundle them in case someone has objections about just one specific type.

@Fidget-Spinner
Copy link
Member

We had a similar discussion some time back for callable, and there was some push back #86268

@sobolevn
Copy link
Member Author

sobolevn commented Sep 1, 2023

callable is a function, map is a type.

>>> type(map)
<class 'type'>
>>> type(callable)
<class 'builtin_function_or_method'>

Plus, Callable is a special form in typing.py, while map[T] is just a regular generic alias.

So, I don't think that this is the same problem.

@serhiy-storchaka
Copy link
Member

What does it mean and in what cases the type of the map iterator cannot be derived from types of map() arguments?

@sobolevn
Copy link
Member Author

sobolevn commented Sep 1, 2023

@serhiy-storchaka I am sorry, I don't fully understand your question.

what cases the type of the map iterator cannot be derived from types of map() arguments?

I don't think that this is related to CPython, because runtime does not derive anything at all. Typecheckers have different inference logic, so it is up to them to derive map's type arguments.

@serhiy-storchaka
Copy link
Member

What does it mean for typecheckers and why they need it?

@sobolevn
Copy link
Member Author

sobolevn commented Sep 1, 2023

map can already be used from type checkers' POV: https://mypy-play.net/?mypy=latest&python=3.11&gist=04a3192eecc77928ea6aa60f40604ef0

But, we have to use string 'map[...]' or from __future__ import annotations just to make sure it won't fail in runtime with TypeError.

@serhiy-storchaka
Copy link
Member

You didn't answer the question why you need it.

@AlexWaygood
Copy link
Member

An argument in favour is that this increases consistency with various other iterator classes that have __class_getitem__ defined, such as builtins.enumerate and itertools.chain:

>>> enumerate[str]
enumerate[str]
>>> import itertools
>>> itertools.chain[bytes]
itertools.chain[bytes]

Additionally, adding the method would only cost two lines of C code, so the maintenance burden is not particularly high.

However, I don't think the motivation here is very strong. You say:

Second, while we can treat map[T] as just Iterator[T] in parameter types, we always want to as specific as possible in return types.

However, I can't think of an instance where using map[T] as a return type would give the type checker any more information than Iterator[T]. Instances of builtins.map don't have any attributes that distinguish them from instances of any other iterator class. While it's true that, in general, we should use the most specific annotation possible for a return type, I don't think this really holds true when it comes to iterators. We should take note of this comment at the top of types.py that explains why list_iterator (the type of iter([])) and various other "builtin" iterators are not exposed in the types module:

cpython/Lib/types.py

Lines 6 to 9 in 578ebc5

# Iterators in Python aren't a matter of type but of protocol. A large
# and changing number of builtin types implement *some* flavor of
# iterator. Don't check the type! Use hasattr to check for both
# "__iter__" and "__next__" attributes instead.

Unless instances of an iterator class have some special attributes that set them apart from other iterator objects, I tend to think that people should avoid using any specific iterator class as a return annotation, and just use Iterator[]. The specific type of iterator being returned is generally an implementation detail, and rarely gives the type checker any extra information to work with. In other words, I think it was probably a mistake to add __class_getitem__ to classes such as builtins.enumerate (but it's also not worth it to go through a deprecation cycle and remove them again; they don't do any particular harm).

If we do want to add __class_getitem__ to map, we should also add it to other builtin iterator classes such as zip, filter and reversed: any arguments for or against adding __class_getitem__ to map would also apply to them, I think. It makes no sense to me to only add the method to map, but not the others.

@sobolevn
Copy link
Member Author

sobolevn commented Sep 1, 2023

However, I can't think of an instance where using map[T] as a return type would give the type checker any more information than Iterator[T]

Since we have nominal type system, map and Iterator are different types. They are, especially, treated differently by runtime tools that use annotations, like: Hypothesis, beartype, typeguard, deal, etc.

I general, I think that all type that are generic in typeshed should be generic in runtime. There's no real cost in doing so, but there's a big profit in making the DX pleasant for people who annotate their code.

>>> from __future__ import annotations
>>> import beartype
>>> @beartype.beartype
... def returns_map() -> map[int]:
...     ...
... 
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/typeshed/.venv/lib/python3.11/site-packages/beartype/peps/_pep563.py", line 584, in resolve_pep563
    func_hints_resolved[pith_name] = eval(
                                     ^^^^^
  File "<string>", line 1, in <module>
TypeError: type 'map' is not subscriptable

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/sobolev/Desktop/typeshed/.venv/lib/python3.11/site-packages/beartype/_decor/_cache/cachedecor.py", line 77, in beartype
    return beartype_object(obj, conf)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sobolev/Desktop/typeshed/.venv/lib/python3.11/site-packages/beartype/_decor/decorcore.py", line 197, in beartype_object
    return _beartype_func(  # type: ignore[return-value]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sobolev/Desktop/typeshed/.venv/lib/python3.11/site-packages/beartype/_decor/decorcore.py", line 608, in _beartype_func
    bear_call.reinit(func, conf, **kwargs)
  File "/Users/sobolev/Desktop/typeshed/.venv/lib/python3.11/site-packages/beartype/_check/checkcall.py", line 341, in reinit
    resolve_pep563(
  File "/Users/sobolev/Desktop/typeshed/.venv/lib/python3.11/site-packages/beartype/peps/_pep563.py", line 621, in resolve_pep563
    raise BeartypePep563Exception(
beartype.roar.BeartypePep563Exception: function __main__.returns_map() return PEP 563-postponed type hint 'map[int]' syntactically invalid (i.e., "type 'map' is not subscriptable") under:
~~~~[ GLOBAL SCOPE ]~~~~
{'__name__': '__main__', '__doc__': None, '__package__': None, '__loader__': <class '_frozen_importlib.BuiltinImporter'>, '__spec__': None, '__annotations__': {}, '__builtins__': <module 'builtins' (built-in)>, 'beartype': <module 'beartype' from '/Users/sobolev/Desktop/typeshed/.venv/lib/python3.11/site-packages/beartype/__init__.py'>, 'annotations': _Feature((3, 7, 0, 'beta', 1), None, 16777216)}
~~~~[ LOCAL SCOPE  ]~~~~
{}

But, if others don't agree for some reason: feel free to close, I don't care enough about this.

If we do want to add class_getitem to map, we should also add it to other builtin iterator classes such as zip, filter and reversed

As I said, I think that this is a good thing, but I want to decouple these tasks.
I only have a very limited amount of energy :)

@hauntsaninja
Copy link
Contributor

hauntsaninja commented Sep 1, 2023

I'm a little wary of doing this for map, just because it's "type-ness" is not documented.

I'd be less wary if I thought there was an especially compelling use case, but I haven't any instances of people using map as a type, instead of Iterator (also struggled to find any on grep.app just now). In fact, I think I'm still slightly opposed to the decision to type them as classes in typeshed, see python/typeshed#5145.

Also see faster-cpython/ideas#131. I think if we added __class_getitem__ we might not be able to make that change.

Of course, this stops mattering in a PEP 718 world.

@AlexWaygood
Copy link
Member

AlexWaygood commented Sep 1, 2023

Since we have nominal type system, map and Iterator are different types. They are, especially, treated differently by runtime tools that use annotations, like: Hypothesis, beartype, typeguard, deal, etc.

You've given an example where a function that is annotated as returning map at runtime would currently cause problems for beartype, but you've yet to give an example where annotating a function as returning map[T] would be demonstrably more correct, and would provide more information to the type checker, than simply annotating the function returning a map instance as returning Iterator[T]. As I mentioned above, in my experience, when a function returns an iterator object, the specific class of the iterator is usually an implementation detail, so the better course of action is usually to annotate the function as returning Iterator[T] rather than map[T].

Would beartype/Hypothesis/typeguard struggle with a returns_map function annotated like this? If so, that seems like a limitation of those tools to me, rather than something we need to fix in CPython:

from __future__ import annotations
from collections.abc import Iterator

def returns_map() -> Iterator[int]:
    return map(len, [range(42)])

@JelleZijlstra
Copy link
Member

I think if PEP-718 is implemented the case for making map() subscriptable becomes stronger.

@JelleZijlstra
Copy link
Member

I'd like to punt the decision here until PEP 718 is resolved. If it is accepted, we should also add subscript support to map(). If it is rejected, we probably shouldn't.

map is technically a class, but in terms of how it's used it's much more like a function, so I expect use cases for subscripting it mirror use cases for subscripting generic functions.

If we make map generic at runtime, we should also do it for filter and reversed, and probably some iterators in itertools. Currently among the four major builtin iterators, only enumerate is generic, but filter, map, and reversed are not. I don't understand how enumerate is different from the others; seems like it was made generic in #19421 without much focused discussion.

@picnixz
Copy link
Member

picnixz commented Sep 9, 2023

My 2 cents:

  • I didn't even know that enumerate was subscriptable. I don't know why the latter should be since it's actually somewhat equivalent to zip(count(start), it) and zip is not subscriptable neither.

  • Having a map[T] may expose some implementation detail that may be needed. Like "hey, here I used a map and not some other construction". But I think that rather having it subscriptable, we should actually not have allowed enumerate to be subscriptable in the first time. AFAICT, enumerate doesn't expose more than what Iterator[tuple[int, T]] exposes, so it's only syntactic sugar for it to be subscriptable (like, it somehow reduces the number of characters to write and make it more readable, so maybe that was why it was made subscriptable in the first place). The same holds for map, filter, reversed and technically speaking, zip as well, although the gain in characters is somewhat arguable. None of them are subscriptable in the first place, so I would say that we shouldn't make map subscriptable if the others aren't.

If PEP 718 is accepted, then all built-ins that are classes but are used like functions should be made subscriptable (namely all that were mentioned + zip).

Otherwise, if we want to stress that something was created using those builtins, I think the user should create something like that:

from __future__ import annotations

from collections.abc import Iterator
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from typing import TypeAlias

MapView: TypeAlias = Iterator

# or for versions supporting it:
# type MapView = Iterator

and use it in stubs or if TYPE_CHECKING guarded blocks. While objects annotated with this type are not guaranteed to pass isinstance(it, map) checks, I don't think it's reasonable to actually perform such checks in the first place (at least I don't have any usecase where you need to do this).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-typing type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

7 participants