typing.Reversible doesn't fully support the reversing protocol

[serhiy-storchaka]

typing.Reversible doesn't work with types that are supported by reserved() but don't have the __reversed__ method. E.g. tuple, str, bytes, bytearray, and array.

>>> issubclass(tuple, typing.Reversible)
False
>>> issubclass(str, typing.Reversible)
False
>>> issubclass(bytes, typing.Reversible)
False
>>> issubclass(bytearray, typing.Reversible)
False
>>> issubclass(array.array, typing.Reversible)
False 

The reversing protocol as well as the iterating protocol is supported not only by special method, but implicitly if the class has implemented __getitem__ and __len__ methods.

>>> class Counter(int):
...   def __getitem__(s, i): return i
...   def __len__(s): return s
... 
>>> list(reversed(Counter(10)))
[9, 8, 7, 6, 5, 4, 3, 2, 1, 0]
>>> issubclass(Counter, typing.Reversible)
False

[gvanrossum]

Yeah, the _ProtocolMeta metaclass that implements the structural check is pretty simple-minded. However, the BDFL-delegate for PEP 484 decreed that the classes in typing should not support isinstance() or issubclass() at all. I ripped out isinstance() support, but ripping out issubclass() support is still on the TODO list (see #136). So I'm not sure it's worth adding this.

The place where this really should be implemented is in the type checker, e.g. mypy (https://github.com/JukkaL/mypy). I don't know whether mypy currently supports this, but it's worth trying and searching its issue tracker. (I would, but I'm home sick and can't concentrate very long.)

[abarnert]

This is the same problem as Iterable being defined as "has __iter__" instead of "has __iter__ or implements the old-style sequence protocol". As far as I can tell, the only reason Iterable passes such types today is that they're all registered with collections.abc.Sequence, which is a subclass of collections.abc.Iterable, and typing.Iterable has extra=collections.abc.Iterable attached and defers to the ABC to do the check. (I'm not sure if/how #136 will affect that.) So the obvious way to fix this problem seems to be doing the equivalent thing that makes Iterable work:

• Add collections.abc.Reversible, which is a subtype of Iterable, and has a __subclasshook__ that checks for __reversed__.
• Change collections.abc.Sequence to be a subtype of Reversible instead of Iterable, so that everything registered as a Sequence or MutableSequence is now Reversible as well as Iterable as far as the ABCs are concerned.
• Change typing.Reversible to add extra=collections.abc.Reversible.
• Any third-party types that want to be Reversible based on the old-style sequence protocol despite not being Sequences could call collections.abc.Reversible.register.

Alternatively, is Reversible even useful in practice? If the only thing it's used for is as a type hint for reverse, and it doesn't get that right, and can't get that right without adding a lot of complexity, maybe it just shouldn't be there.

[gvanrossum]

The type checker (e.g. mypy) is independent from the issubclass()
implementation in typing.py. As I wrote in a different thread we shouldn't
even have the latter, per decree from the BDFL-delegate for PEP 484. AFAICT
mypy does the right thing for Reversible and those builtin types.

[gvanrossum]

IOW Reversible does get things right in practice (in the type checker).

FWIW, maybe we should try to deprecate supporting iteration using the old-style protocol? It's really a very old backwards compatibility measure (from when iterators were first introduced). Then eventually we could do the same for reversing using the old-style protocol.

[abarnert]

The last time deprecating the old-style protocol came up, Stephen d'Aprano strongly objected (http://article.gmane.org/gmane.comp.python.ideas/23369/match=old+sequence+protocol), citing an earlier objection by Raymond Hettinger (which I don't have a link for).

[gvanrossum]

On Sat, Dec 26, 2015 at 6:24 PM, Andrew Barnert notifications@github.com
wrote:

The last time deprecating the old-style protocol came up, Stephen d'Aprano
strongly objected (
http://article.gmane.org/gmane.comp.python.ideas/23369/match=old+sequence+protocol),
citing an earlier objection by Raymond Hettinger (which I don't have a link
for).

Hm. The example given there is a toy though. Something with a getitem
that maps its argument to its square might as well be a mapping. I really
think it's time to slowly let go of this (no need to rush into removing
support, but we could still frown upon its use).

--Guido van Rossum (python.org/~guido)

[serhiy-storchaka]

The issue is fixed for tuple, str, bytes and bytearray. But the issubclass() check still doesn't work for array.array and sample Counter class.

[ilevkivskyi]

@serhiy-storchaka Yes, I also noticed this some time ago. But now typing.Reversible just defers all checks to collections.abc.Reversible, so that if we fix the latter, then this will be automatically fixed.

[srittau]

If this is still an issue, it should be reported to bpo.