Skip to content

Explicitly mark subclasses of collections.abc protocols as abstract base classes #11266

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.

Sign up for GitHub

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

Jump to bottom
Closed
gracepetryk opened this issue Jan 10, 2024 · 2 comments
Closed

Explicitly mark subclasses of collections.abc protocols as abstract base classes #11266

gracepetryk opened this issue Jan 10, 2024 · 2 comments
Labels
stubs: improvement Improve/refactor existing annotations, other stubs issues

Comments

@gracepetryk
Copy link

gracepetryk commented Jan 10, 2024 •
edited
Loading

There are a few types in collection.abc that subclass protocols but are not protocols themselves. For example Sequence is a subclass of Collection and Reversible. It expects subclasses to implement __getitem__ and __len__, the latter being a requirement of the Collection protocol. I think that the stubs for these classes should explicitly subclass abc.ABC so that typecheckers require implementation of all abstract methods when the @final annotation is used.

Currently typecheckers disagree on how to handle these stubs. For this example mypy asks for __getitem__ and __len__ implementations and pyright throws no error:

from collections.abc import Sequence
from typing import final

@final
class Foo(Sequence[int]):
    pass
bash-3.2$ mypy test.py
test.py:5: error: Final class test.Foo has abstract attributes "__getitem__", "__len__"  [misc]
Found 1 error in 1 file (checked 1 source file)
bash-3.2$ pyright test.py
0 errors, 0 warnings, 0 informations

At runtime the types in collections.abc are already abstract base classes:

>>> from collections.abc import Sequence
>>> Sequence.__class__
<class 'abc.ABCMeta'>
@gracepetryk gracepetryk mentioned this issue Jan 10, 2024
Closed
@gracepetryk gracepetryk changed the title Explicitly mark sublcasses of protocols as abstract base classes. Explicitly mark sublcasses of collections.abc protocols as abstract base classes Jan 10, 2024
@gracepetryk gracepetryk changed the title Explicitly mark sublcasses of collections.abc protocols as abstract base classes Explicitly mark subclasses of collections.abc protocols as abstract base classes Jan 10, 2024
@srittau
Copy link
Collaborator

srittau commented Jan 10, 2024

This sounds like something we would have tried before and didn't do for one reason or another. Possibly because we also mark some of the classes as protocols, and that might not interact well with ABCMeta. That said, as we strive to follow the implementation if possible, I agree that this something we should at least try do to – or at least document in the stubs why we don't do it.

Exploratory PR welcome, unless someone else knows why didn't do it before.

@srittau srittau added the stubs: improvement Improve/refactor existing annotations, other stubs issues label Jan 10, 2024
@gracepetryk
Copy link
Author

There's a note in PEP-544 about this that to me sounds like marking all the non-protocol collection.abc classes as ABCs in the stubs would be a good solution. I'll submit a PR doing that a little later today.

Subclassing a protocol class would not turn the subclass into a protocol unless it also has typing.Protocol as an explicit base class. Without this base, the class is “downgraded” to a regular ABC that cannot be used with structural subtyping.

@gracepetryk gracepetryk closed this as not planned Won't fix, can't repro, duplicate, stale May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stubs: improvement Improve/refactor existing annotations, other stubs issues
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@srittau @gracepetryk