typeshed icon indicating copy to clipboard operation
typeshed copied to clipboard

incorrect annotations for `builtins.reversed`

Open jorenham opened this issue 1 year ago • 6 comments

The builtins.reversed.__new__ signature is currently annotated like (Reversible[T] | SupportsLenAndGetItem[T]) -> Iterator[T]: https://github.com/python/typeshed/blob/3d26992d1562a5175fb8fd91ca63817e55feee56/stdlib/builtins.pyi#L1639-L1646

It has two problems:

argument of type SupportsLenAndGetItem[T]

This should return a reversed[T] instance (i.e. Self), instead of "just" an Iterable[T], which is unnecessarily loose.

argument with type that implement __reversed__

This is only handled if the argument's __reversed__ returns an Iterator[T], i.e. arguments that are Reversible[T]. But in reality, reversed(x) is equivalent to x.__reversed__() (if __reversed__ exists). To illustrate:

>>> class Revver[X]:
...     x: X
...     def __init__(self, x: X):
...         self.x = x
...     def __reversed__(self) -> X:
...         return self.x
... 
>>> reversed(Revver(42))
42

However, this case isn't handled because Revver[X] cannot be expressed as a Reversible[?].


So instead, it should look something like this:

class SupportsReversed[T]:
    def __reversed__(self) -> T: ...

class reversed[T](Iterator[T]):
    @overload
    def __new__[R](cls, sequence: SupportsReversed[R], /) -> R: ...
    @overload
    def __new__(cls, sequence: SupportsLenAndGetItem[T], /) -> Self: ...
    def __length_hint__(self) -> int: ...
    # btw, this is why protocols shouldn't be mixed with abstract classes:
    def __iter__(self) -> Self: ...
    def __next__(self) -> T: ...

Let me know if I should make this into PR :)

jorenham avatar Mar 21 '24 16:03 jorenham

This looks right; feel free to send a PR. We previously dealt with reversed() in #10655. It's probably worth adding your examples to the test cases.

JelleZijlstra avatar Mar 21 '24 18:03 JelleZijlstra

Do you have real-world use cases for these changes, or is this just correctness for the sake of correctness? From a practical point of view, it makes sense for type checkers to conmplain about returning an integer in __reversed__, and as far as I can tell, a reversed instance doesn't have anything important that Iterator[_T] lacks.

Akuli avatar Mar 25 '24 19:03 Akuli

@Akuli I was trying to implement await reversed(...) for "async sequences", which aren't (sync) iterators.

jorenham avatar Mar 25 '24 22:03 jorenham

@Akuli I also stumbled upon this issue whilst writing the callback protocols in optype; an opinionated low-level typing library that I've been working on. To be precise, I encountered it in optype.DoesReversed, which is the callable type of reversed (aliased as optype.do_reversed for the sake of completeness).

jorenham avatar Mar 25 '24 22:03 jorenham

@Akuli I just thought of another "cool" example: using reversed on scipy.stats distribution instances (i.e. RV's) to "flip" them, so that e.g. reversed(scipy.stats.gumbel_r()) returns the equivalent of scipy.stats.gumbel_l().

I can imagine that using reversed this way (i.e. reversing the domain of a mathematical-isch function) could also be applyied within e.g. numpy.polynomial, sympy, pymc, etc...

jorenham avatar Mar 25 '24 22:03 jorenham

Alright, this makes more sense to me now :)

Akuli avatar Mar 26 '24 00:03 Akuli