This repository has been archived by the owner. It is now read-only.

Accept generator created in Cython #233

Closed
methane opened this Issue Apr 19, 2015 · 5 comments

Comments

Projects
None yet
5 participants
@methane
Member

methane commented Apr 19, 2015

Problem was reported here: https://groups.google.com/forum/#!topic/cython-users/g146SZHxRyM
I think supporting Cython is very important for asyncio performance.

@gvanrossum

This comment has been minimized.

Show comment
Hide comment
@gvanrossum

gvanrossum Apr 20, 2015

Member

Agreed, I would gladly accept patches. I'm not a Cython user myself so I can't fix this without help.

Member

gvanrossum commented Apr 20, 2015

Agreed, I would gladly accept patches. I'm not a Cython user myself so I can't fix this without help.

@GMLudo

This comment has been minimized.

Show comment
Hide comment
@GMLudo

GMLudo Apr 20, 2015

Member

Hi @methane, thank you for the link.
On the mailing-list, Stefan seems to find a solution: cython/cython@4af4244
I've played my role of messenger: I've invited him to open a pull request.

Member

GMLudo commented Apr 20, 2015

Hi @methane, thank you for the link.
On the mailing-list, Stefan seems to find a solution: cython/cython@4af4244
I've played my role of messenger: I've invited him to open a pull request.

@scoder

This comment has been minimized.

Show comment
Hide comment
@scoder

scoder Apr 20, 2015

I had already created a CPython ticket for this:

https://bugs.python.org/issue24004

Here's what Cython currently (as in "since yesterday") does in order to circumvent the type checks in asyncio and inspect:

https://github.com/cython/cython/blob/4af42443bd37f6207143f8870904f780a65bb3e3/Cython/Utility/Generator.c#L824

https://github.com/cython/cython/blob/4af42443bd37f6207143f8870904f780a65bb3e3/Cython/Utility/Generator.c#L906

It's not clear to me what the best fix is here, but my gut feeling tells me that the correct way to deal with this is to create a Generator ABC and do the type checking based on that. A "Generator" is an object that implements the Generator protocol, not necessarily something of type types.GeneratorType. Special casing that type is fine, obviously, but excluding everything else isn't.

The fact that this crash bug here went unnoticed since Py3.3 makes it obvious, though, that no-one has ever tried this at the C level:

https://bugs.python.org/issue23996

scoder commented Apr 20, 2015

I had already created a CPython ticket for this:

https://bugs.python.org/issue24004

Here's what Cython currently (as in "since yesterday") does in order to circumvent the type checks in asyncio and inspect:

https://github.com/cython/cython/blob/4af42443bd37f6207143f8870904f780a65bb3e3/Cython/Utility/Generator.c#L824

https://github.com/cython/cython/blob/4af42443bd37f6207143f8870904f780a65bb3e3/Cython/Utility/Generator.c#L906

It's not clear to me what the best fix is here, but my gut feeling tells me that the correct way to deal with this is to create a Generator ABC and do the type checking based on that. A "Generator" is an object that implements the Generator protocol, not necessarily something of type types.GeneratorType. Special casing that type is fine, obviously, but excluding everything else isn't.

The fact that this crash bug here went unnoticed since Py3.3 makes it obvious, though, that no-one has ever tried this at the C level:

https://bugs.python.org/issue23996

@methane

This comment has been minimized.

Show comment
Hide comment
@methane

methane Apr 20, 2015

Member

I think adding Coroutine ABC to somewhere is good way to solve this problem.
It may be in the stdlib. But I think we should wait discussion of async/await syntax.

For now, how about asyncio.coroutines.Coroutine ?

from abc import ABCMeta, abstractmethod
import types


class Coroutine(metaclass=ABCMeta):
    def __iter__(self):
        return self

    def __next__(self):
        raise StopIteration

    @abstractmethod
    def send(self):
        raise StopIteration


Coroutine.register(types.GeneratorType)
Coroutine.register(CoroWrapper)


def iscoroutine(obj):
    """Return True if obj is a coroutine object."""
    return isinstance(obj, Coroutine)
Member

methane commented Apr 20, 2015

I think adding Coroutine ABC to somewhere is good way to solve this problem.
It may be in the stdlib. But I think we should wait discussion of async/await syntax.

For now, how about asyncio.coroutines.Coroutine ?

from abc import ABCMeta, abstractmethod
import types


class Coroutine(metaclass=ABCMeta):
    def __iter__(self):
        return self

    def __next__(self):
        raise StopIteration

    @abstractmethod
    def send(self):
        raise StopIteration


Coroutine.register(types.GeneratorType)
Coroutine.register(CoroWrapper)


def iscoroutine(obj):
    """Return True if obj is a coroutine object."""
    return isinstance(obj, Coroutine)
@1st1

This comment has been minimized.

Show comment
Hide comment
@1st1

1st1 May 13, 2015

Member

@scoder You can now use collections.abc.Coroutine for your purposes (3a09a93). Closing this issue.

Member

1st1 commented May 13, 2015

@scoder You can now use collections.abc.Coroutine for your purposes (3a09a93). Closing this issue.

@1st1 1st1 closed this May 13, 2015

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.