Skip to content
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.

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

bpo-31861: Add aiter and anext to builtins #23847

Open
wants to merge 28 commits into
base: master
from
Open

Conversation

@jab
Copy link
Contributor

@jab jab commented Dec 18, 2020

This is the C implementation for bpo-31861 requested as an alternative to the Python implementation provided in #8895.

For a more direct translation of this into Python (in case it makes reviewing easier), see jab@ce35092.

Patch by @justin39, @lordmauve, and me.

https://bugs.python.org/issue31861

@the-knights-who-say-ni
Copy link

@the-knights-who-say-ni the-knights-who-say-ni commented Dec 18, 2020

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@justin39

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

Objects/iterobject.c Outdated Show resolved Hide resolved
jab added 3 commits Dec 18, 2020
Doc/library/functions.rst Outdated Show resolved Hide resolved
/* Can we raise this at this point, or do we need to return an awaitable
* that raises it? */
PyErr_SetNone(PyExc_StopAsyncIteration);
Comment on lines +368 to +370

This comment has been minimized.

@eric-wieser

eric-wieser Dec 19, 2020
Contributor

I assume this comment is for reviewers, and should be removed / addressed before merging? If so, leaving this comment as a marker for someone who knows more.

Objects/iterobject.c Outdated Show resolved Hide resolved
@Fidget-Spinner
Copy link
Contributor

@Fidget-Spinner Fidget-Spinner commented Dec 19, 2020

IMO, this looks like it should be in whatsnew too under this section. It's not everyday two new builtins get added :). Maybe something like

* Two new builtin functions -- :func:`aiter` and :func:`anext` have been added
  to provide asynchronous counterparts to :func:`iter` and :func:`anext` 
  respectively.
  (Contributed by Joshua Bronson, Justin Wang and Daniel Pope in :issue:`31861`.)
Copy link
Contributor

@nanjekyejoannah nanjekyejoannah left a comment

I agree, a whatsnew entry is required in this case. I will ping @1st1 for a look.

@nanjekyejoannah nanjekyejoannah requested a review from 1st1 Dec 19, 2020
Objects/abstract.c Outdated Show resolved Hide resolved
@lordmauve

This comment has been minimized.

Copy link
Contributor

@lordmauve lordmauve commented on 8b8a689 Dec 21, 2020

This isn't right, is it? I though argument clinic gave us NULL if we make that the default?

This comment has been minimized.

Copy link
Author

@justin39 justin39 replied Dec 21, 2020

I changed the default to None, but that might not be right... we do want NULL, I think, but for some reason AC doesn't like that

This comment has been minimized.

Copy link
Author

@justin39 justin39 replied Dec 21, 2020

Yeah this is wrong - NULL is the right value, but it means we get an invalid signature aiter($module, aiterable, sentinel=<unrepresentable>, /); I'll revert it back today and try to figure out the signature business along the way

@ghost
ghost approved these changes Dec 22, 2020
Copy link

@ghost ghost left a comment

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.