Skip to content

Using types classes over collections.abc's bases #1480

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
Gobot1234 opened this issue Oct 1, 2023 · 11 comments
Open

Using types classes over collections.abc's bases #1480

Gobot1234 opened this issue Oct 1, 2023 · 11 comments
Labels
topic: other Other topics not covered

Comments

@Gobot1234
Copy link
Contributor

Gobot1234 commented Oct 1, 2023

Currently collections.abc.Generator/Coroutine/AsyncGenerator etc. have attributes that don't have to exist at runtime. I think it'd be wise to switch recommending to using the types concrete classes where possible over the abstract versions which have a smaller interface because having this weird sort of duplication where the 2 are basically the same is a wrinkle that's bitten me recently. To do this I think a few things need to happen:

  1. make the classes in collections.abc.pyi Protocols that should be based on their runtime implementation (they implement custom __subclasshook__) and not the concrete types. (Make collections.abcs more consistent with runtime implementation typeshed#10816)
  2. make the types classes subscriptable at runtime. (gh-110209: Add __class_getitem__ for generator and coroutine cpython#110212)
  3. make type checkers infer things like def foo(): yield as types.GeneratorType and not just collections.abc.Generator.

(as a small aside it might be nice to move the type implementations from typing to collections.abc soon)

refs: microsoft/pyright#6053

@gvanrossum
Copy link
Member

it'd be wise to switch recommending to using the types concrete classes where possible

I can't follow this sentence. Could you elaborate, maybe by mentioning the actual attributes you're referring to.

@Gobot1234
Copy link
Contributor Author

Gobot1234 commented Oct 1, 2023

Currently collections.abc.Generator defines itself as having these extra properties

    @property
    def gi_code(self) -> CodeType: ...
    @property
    def gi_frame(self) -> FrameType: ...
    @property
    def gi_running(self) -> bool: ...
    @property
    def gi_yieldfrom(self) -> Generator[Any, Any, Any] | None: ...

which aren't required by the actual runtime subclass check (which is just

            _check_methods(C, '__iter__', '__next__', 'send', 'throw', 'close')

)

@gvanrossum
Copy link
Member

Oh, this is a typeshed issue. You should probably close this issue here and rely on the typeshed issue you just opened.

@Gobot1234
Copy link
Contributor Author

It's not purely typeshed related because it does require changes to type checkers as well and things at runtime

@Gobot1234 Gobot1234 changed the title types over collections.abc Using types classes over collections.abc's bases Oct 1, 2023
@Gobot1234 Gobot1234 changed the title Using types classes over collections.abc's bases Using types classes over collections.abc's bases Oct 1, 2023
@gvanrossum
Copy link
Member

What runtime changes do you propose?

@Gobot1234
Copy link
Contributor Author

  1. make the types classes subscriptable at runtime.
Python 3.13.0a0 (heads/more-pydoc-str-dirty:a6fedf2a0a, Oct  1 2023, 11:49:10) [Clang 14.0.3 (clang-1403.0.22.14.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import types
>>> types.CoroutineType[None, None, int]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: type 'coroutine' is not subscriptable

@erictraut
Copy link
Collaborator

I don't think that any runtime changes are required to address the problem you're talking about here. We simply need a protocol defined somewhere in typeshed (e.g. in the _typeshed module) that type checkers can use for this purpose. The SupportsKeysAndGetItem is analogous and is used by both pyright and mypy.

@Gobot1234
Copy link
Contributor Author

I think the protocol partly solves the problem but there is a bigger issue underlying all of this. The runtime changes at least to me seem like a good idea because the types classes are subscriptable at type time just not at runtime currently

@Gobot1234
Copy link
Contributor Author

@erictraut would you mind explaining why you think doing this is a bad idea?

@erictraut
Copy link
Collaborator

erictraut commented Oct 12, 2023

The classes defined in types are runtime details. Unless you're writing tooling-related code that needs access to these details (e.g. a runtime type checker or debugger), you should not use these classes in type annotations. This is already a point of confusion for newcomers to typing, and my blanket guidance is "if you're importing a type from the types module and using it in a type annotation, you're probably doing something wrong." If we start to promote the use of some of the classes from the types module, it will create significant confusion. It will also require type checkers to expand existing special-case logic that knows about classes in the typing and collections.abc modules.

If there are problems with the existing ABCs defined in collections.abc, let's fix those rather than doing something more radical.

@Gobot1234
Copy link
Contributor Author

Gobot1234 commented Oct 12, 2023

I'd propose enforcing that the return types of things that are at runtime in the types module as their collections.abc counterparts and then by structural type checks the types versions should be fine to return if you want the specific "correct" types. That shouldn't at least to my knowledge require much more special casing than currently present, which is why I don't see this change being problematic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: other Other topics not covered
Projects
None yet
Development

No branches or pull requests

3 participants