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

add aiter() and anext() functions #76042

Closed
sorcio mannequin opened this issue Oct 24, 2017 · 25 comments
Closed

add aiter() and anext() functions #76042

sorcio mannequin opened this issue Oct 24, 2017 · 25 comments
Labels
3.10 stdlib

Comments

@sorcio
Copy link
Mannequin

@sorcio sorcio mannequin commented Oct 24, 2017

BPO 31861
Nosy @gvanrossum, @vstinner, @basak, @jab, @sorcio, @dimaqq, @1st1, @JelleZijlstra, @pablogsal, @isidentical, @belm0, @jack1142, @sweeneyde
PRs
  • #8895
  • #23847
  • #25004
  • #25005
  • #25008
  • Files
  • aiter_comp.py: tentative Python implementation
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-03-24.01:43:07.674>
    created_at = <Date 2017-10-24.13:26:15.573>
    labels = ['library', '3.10']
    title = 'add aiter() and anext() functions'
    updated_at = <Date 2021-04-07.22:59:05.539>
    user = 'https://github.com/sorcio'

    bugs.python.org fields:

    activity = <Date 2021-04-07.22:59:05.539>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-03-24.01:43:07.674>
    closer = 'pablogsal'
    components = ['Library (Lib)']
    creation = <Date 2017-10-24.13:26:15.573>
    creator = 'davide.rizzo'
    dependencies = []
    files = ['47242']
    hgrepos = []
    issue_num = 31861
    keywords = ['patch']
    message_count = 25.0
    messages = ['304910', '304923', '304924', '304926', '305107', '319519', '319520', '323989', '323997', '324009', '337061', '337135', '375633', '380408', '381832', '383327', '383441', '389414', '389417', '389420', '389422', '389426', '389427', '390402', '390495']
    nosy_count = 15.0
    nosy_names = ['gvanrossum', 'vstinner', 'rb', 'jab', 'davide.rizzo', 'jmehnle', 'Dima.Tisnek', 'yselivanov', 'JelleZijlstra', 'pablogsal', 'BTaskaya', 'Eric Wieser', 'John Belmonte', 'jack1142', 'Dennis Sweeney']
    pr_nums = ['8895', '23847', '25004', '25005', '25008']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue31861'
    versions = ['Python 3.10']

    @sorcio
    Copy link
    Mannequin Author

    @sorcio sorcio mannequin commented Oct 24, 2017

    PEP-525 suggested that adding aiter() and anext() would need to wait until async __aiter__ is dropped in 3.7. bpo-31709 solved that, so now it would be possible to add them.

    @sorcio sorcio mannequin added 3.7 stdlib labels Oct 24, 2017
    @1st1
    Copy link
    Member

    @1st1 1st1 commented Oct 24, 2017

    Guido, do we need a PEP to add aiter() and anext() builtins?

    @gvanrossum
    Copy link
    Member

    @gvanrossum gvanrossum commented Oct 24, 2017

    do we need a PEP to add aiter() and anext() builtins?

    No, just this tracker issue, a PR and a reviewer. (Sorry, I can't review
    CPython code myself any more.)

    @1st1
    Copy link
    Member

    @1st1 1st1 commented Oct 24, 2017

    No, just this tracker issue, a PR and a reviewer. (Sorry, I can't review
    CPython code myself any more.)

    Alright, I'll work on a PR after PEP 55x.

    @sorcio
    Copy link
    Mannequin Author

    @sorcio sorcio mannequin commented Oct 27, 2017

    I attempted to make a Python implementation (attached) to use in my code. There are a few questions in the comments.

    One of the complications is the async equivalent of next with two arguments like next(iterator, default). It cannot return the result of __anext__() because it needs to catch StopAsyncIteration. So it should return an awaitable wrapper instead (in my Python code this is rendered as a coroutine). A secondary question is whether the default value should be returned as it is passed, or awaited on.

    @JelleZijlstra
    Copy link
    Member

    @JelleZijlstra JelleZijlstra commented Jun 14, 2018

    Do these really need to be builtins?

    They seem too specialized to be widely useful; I've personally never needed them in any async code I've written. It would make more sense to me to put them in a module like operators.

    @1st1
    Copy link
    Member

    @1st1 1st1 commented Jun 14, 2018

    Do these really need to be builtins?

    We're only beginning to see async iterators being used in the wild, so we can't have a definitive answer at this point.

    They seem too specialized to be widely useful; I've personally never needed them in any async code I've written. It would make more sense to me to put them in a module like operators.

    I think putting them to the operators module makes sense, at least for 3.8. Do you want to work on a pull request?

    @jab
    Copy link
    Mannequin

    @jab jab mannequin commented Aug 24, 2018

    Updating the issue title to reflect the decision to add these to the operator module rather than to built-ins.

    Submitted a PR here: #8895

    @jab jab mannequin changed the title aiter() and anext() built-in functions add aiter() and anext() functions to operator module Aug 24, 2018
    @1st1
    Copy link
    Member

    @1st1 1st1 commented Aug 24, 2018

    Great, thanks! There's someone working on a pr to add them as builtins (so that pr is in C). I'll have to take a look when i'm back from vacation at that pr too, before we can make a decision.

    I've been to europython, pycon ru, and pybay conferences recently and the number of people who want these as builtins was surprisingly high.

    @jab
    Copy link
    Mannequin

    @jab jab mannequin commented Aug 24, 2018

    Interesting, thanks Yury!

    I only saw the discussion here which implied these wouldn't go directly into builtins for 3.8 (and I searched here and in GitHub, and couldn't find the PR you mentioned either), so I'm curious if that was tracked somewhere. It'd be unfortunate if the work I did on that PR couldn't be used, but I'd be even happier to have these as builtins.

    Thanks again for reviewing when you're back, and have a wonderful vacation in the meantime!

    @dimaqq
    Copy link
    Mannequin

    @dimaqq dimaqq mannequin commented Mar 4, 2019

    https://www.python.org/dev/peps/pep-0525/#aiter-and-anext-builtins kinda promised aiter and anext built-ins.

    This ticket seems idle.

    Perhaps it's time for the decider club to either remove that language from PEP-525 or make a plan for aiter/anext?

    @dimaqq dimaqq mannequin added 3.8 and removed 3.7 labels Mar 4, 2019
    @jab
    Copy link
    Mannequin

    @jab jab mannequin commented Mar 4, 2019

    If the deciders prefer to have these in the operator module for 3.8 (as Yury and Jelle requested above), my PR from a few months ago which implements this (#8895) can still be merged with no conflicts. I don't think any other changes to that patch are requested before it can be merged (i.e. it's only stalled on the decision whether to add these to builtins or operator), but I can still make time to address any new requested code changes if these are to go in operator.

    If these are to go in builtins instead, @nanjekyejoannah has volunteered to pick that up. So it seems like this can move forward one way or the other once we have a decision on operator vs. builtins.

    @dimaqq
    Copy link
    Mannequin

    @dimaqq dimaqq mannequin commented Aug 19, 2020

    Might as well re-target for 3.10 as 3.9 seems feature-complete now.

    @dimaqq dimaqq mannequin added 3.10 and removed 3.8 labels Aug 19, 2020
    @belm0
    Copy link
    Mannequin

    @belm0 belm0 mannequin commented Nov 5, 2020

    Adding this would be a nice compliment to aclosing(), which was already merged for 3.10.

    Otherwise, witness the ugliness of using aclosing() with an async iterable object:

        async with aclosing(foo.__aiter__()) as agen:
            async for item in agen:
                ...

    I'd like:

        async with aclosing(aiter(foo)) as agen:
            async for item in agen:
                ...

    @jab
    Copy link
    Mannequin

    @jab jab mannequin commented Nov 25, 2020

    Nice to see there is still interest in this from someone else! Thanks, John. Are any core developers still interested in this?

    If I can get a new PR together that adds C implementations of aiter and anext to builtins, would a committer still be interested in reviewing the patch?

    A week from Friday, I'll have a rare and precious opportunity to spend the day contributing to open source. I have a bunch of different things I could work on, but would work on this if there is still interest. Thanks and hope this finds you all well.

    @jab
    Copy link
    Mannequin

    @jab jab mannequin commented Dec 18, 2020

    Please see #23847 for the C implementation of aiter and anext added to builtins, as requested.

    @jab jab mannequin changed the title add aiter() and anext() functions to operator module add aiter() and anext() functions Dec 18, 2020
    @isidentical
    Copy link
    Sponsor Member

    @isidentical isidentical commented Dec 20, 2020

    I don't have anything to add to this beside the name choice is safe and won't clash with anything (but honestly I would prefer it to be discussed on the ML before implementing something after 3 years). I checked a limited dataset to search for aiter and only found examples from 2 different projects. Elastic have something for themselves and the other usages are coming from the tests of aioitertools.
    https://github.com/elastic/elasticsearch-py/blob/5fe9ff721ce493fbf2fc8b94d5ab02fc7e55fd5a/elasticsearch/_async/helpers.py#L85-L96

    @gvanrossum
    Copy link
    Member

    @gvanrossum gvanrossum commented Mar 23, 2021

    New changeset f0a6fde by Joshua Bronson in branch 'master':
    bpo-31861: Add aiter and anext to builtins (bpo-23847)
    f0a6fde

    @pablogsal
    Copy link
    Member

    @pablogsal pablogsal commented Mar 23, 2021

    New changeset d969202 by Pablo Galindo in branch 'master':
    bpo-31861: Complete the C-API docs for PyObject_GetAiter and PyAiter_Check (GH-25004)
    d969202

    @pablogsal
    Copy link
    Member

    @pablogsal pablogsal commented Mar 24, 2021

    New changeset 919d42d by Pablo Galindo in branch 'master':
    bpo-31861: Fix possible crash in PyAnextAwaitable_New (GH-25005)
    919d42d

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Mar 24, 2021

    I reopen the issue.

    The commit f0a6fde introduced a reference leak:

    $ ./python -m test -R 3:3 test_asyncgen

    0:00:00 load avg: 4.75 Run tests sequentially
    0:00:00 load avg: 4.75 [1/1] test_asyncgen
    beginning 6 repetitions
    123456
    ......
    test_asyncgen leaked [72, 72, 72] references, sum=216
    test_asyncgen leaked [30, 30, 30] memory blocks, sum=90
    test_asyncgen failed

    == Tests result: FAILURE ==

    1 test failed:
    test_asyncgen

    Total duration: 6.0 sec
    Tests result: FAILURE

    @vstinner vstinner reopened this Mar 24, 2021
    @pablogsal
    Copy link
    Member

    @pablogsal pablogsal commented Mar 24, 2021

    New changeset a02683a by Pablo Galindo in branch 'master':
    bpo-31861: Fix reference leak in builtin_anext_impl() (GH-25008)
    a02683a

    @pablogsal
    Copy link
    Member

    @pablogsal pablogsal commented Mar 24, 2021

    Fixed by PR25008

    @sweeneyde
    Copy link
    Member

    @sweeneyde sweeneyde commented Apr 7, 2021

    A bug was reported in anext(): https://bugs.python.org/issue43751

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Apr 7, 2021

    I proposed to PR 25266 to rename PyAnextAwaitable_Type to _PyAnextAwaitable_Type, and to initialize the type at Python startup: can someone please have a look?

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 stdlib
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants