◐ Shell
reader mode source ↗
Skip to content

bpo-43751: Fix anext() bug where it erroneously returned None#25238

Merged
pablogsal merged 16 commits into
python:masterfrom
sweeneyde:fix_anext
Apr 11, 2021
Merged

bpo-43751: Fix anext() bug where it erroneously returned None#25238
pablogsal merged 16 commits into
python:masterfrom
sweeneyde:fix_anext

Conversation

@sweeneyde

@sweeneyde sweeneyde commented Apr 7, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@pablogsal

pablogsal commented Apr 7, 2021

Copy link
Copy Markdown
Member

There are a bunch of problems in the CI. The main one is:

======================================================================
ERROR: test_async_gen_anext (test.test_asyncgen.AsyncGenAsyncioTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/cpython/cpython/Lib/test/test_asyncgen.py", line 401, in test_async_gen_anext
    res = self.loop.run_until_complete(consume())
  File "/Users/runner/work/cpython/cpython/Lib/asyncio/base_events.py", line 641, in run_until_complete
    return future.result()
  File "/Users/runner/work/cpython/cpython/Lib/test/test_asyncgen.py", line 399, in consume
    results.append(await anext(g, 'buckle my shoe'))
TypeError: anext() argument was not async iterable.

----------------------------------------------------------------------

and there is also some trailing whitespace. Could you check these?

@sweeneyde

sweeneyde commented Apr 7, 2021

Copy link
Copy Markdown
Member Author

This should probably have been a draft PR. I wrote a test for the erroneous None returns, then fixed it. And then I applied the same test to async iterables that are not generators, and that is still failing.

@sweeneyde

sweeneyde commented Apr 8, 2021

Copy link
Copy Markdown
Member Author

This should be ready for review now.

@pablogsal pablogsal requested review from 1st1 and gvanrossum April 8, 2021 23:55
jab
jab reviewed Apr 9, 2021
View reviewed changes

@jab jab left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

Thanks for catching this!

@gvanrossum

gvanrossum commented Apr 9, 2021

Copy link
Copy Markdown
Member

Side comment on the PR title: please don't use "should no longer". Say something like "Fix anext() bug where it erroneously returned None".

@sweeneyde @jab
Update Objects/iterobject.c …
4039170 
Fix inconsistent variable names in comment

Co-authored-by: Joshua Bronson <jabronson@gmail.com>
@sweeneyde sweeneyde changed the title bpo-43751: anext() should no longer erroneously return None. Apr 9, 2021
@gvanrossum

gvanrossum commented Apr 10, 2021

Copy link
Copy Markdown
Member

I'm requesting that someone else reviews this instead of me. @pablogsal? @1st1?

@sweeneyde
Add null check for new_awaitable; remove null check for am_await beca… …
6a6fa66 
…use awaitable is always a coroutine in that case.
@sweeneyde

sweeneyde commented Apr 11, 2021

Copy link
Copy Markdown
Member Author

./python -m test test_asyncgen -R3:3 now passes

@pablogsal pablogsal merged commit dfb4532 into python:master Apr 11, 2021
@pablogsal

pablogsal commented Apr 11, 2021

Copy link
Copy Markdown
Member

Thanks for the PR @sweeneyde

@sweeneyde sweeneyde deleted the fix_anext branch December 19, 2021 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal

@1st1 1st1 Awaiting requested review from 1st1

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

+1 more reviewer

@jab jab jab left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sweeneyde @pablogsal @gvanrossum @jab @the-knights-who-say-ni @bedevere-bot