omnilib / aioitertools

itertools and builtins for AsyncIO and mixed iterables
https://aioitertools.omnilib.dev
MIT License
241 stars 23 forks source link

isinstance check order of iter method #11

Closed thehesiod closed 4 years ago

thehesiod commented 5 years ago

iter is currently defined as such:

def iter(itr: AnyIterable[T]) -> AsyncIterator[T]:
    if isinstance(itr, AsyncIterator):
        return itr

    if isinstance(itr, AsyncIterable):
        return itr.__aiter__()

This causes an issue for classes defined as such:

class Foo:
    def __aiter__(self):
        return self.__anext__()

    @async_generator
    async def __anext__(self):
       ...

since it won't use the object returned by __aiter__() as both isinstance checks are true. I believe these two checks should be reversed with a comment why they are in the order they are.

amyreese commented 5 years ago

My primary concern with this is that reversing the order will mean that existing iterators will never be used, because iterators are also iterables, and would then always call itr.__aiter__(). You can't have an AsyncIterator without implementing __aiter__ which then means it's also an AsyncIterable:

from typing import AsyncIterator, AsyncIterable

class Aiter:
    def __aiter__(self):
        return self

class Anext:
    async def __anext__(self):
        raise AsyncStopIteration

class Aboth(Aiter, Anext):
    pass

for c in (Aiter, Anext, Aboth):
    a = c()
    itr = isinstance(a, AsyncIterator)
    ita = isinstance(a, AsyncIterable)
    print(a.__class__, itr, ita)
<class '__main__.Aiter'> False True
<class '__main__.Anext'> False False
<class '__main__.Aboth'> True True

More to the point, I believe that your example of Foo.__anext__ is against the specification of PEP 492 (emphasis mine):

An asynchronous iterator object must implement an __anext__ method (or, if defined with CPython C API, tp_as_async.am_anext slot) returning an awaitable.

An asynchronous generator is not itself an awaitable, and therefore doesn't satisfy the API of __anext__. For your specific example, you should instead be implementing your class like this:

class Foo:
    def __aiter__(self):
        return self._iterator()

    async def _iterator(self):
        yield ...

In this case, the async generator object returned by calling self._iterator() will itself be an AsyncIterator with its own __anext__ method, while allowing your class Foo to be a valid AsyncIterable.