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

don't prefetch next item in loop context #1101

Merged
merged 2 commits into from Nov 8, 2019
Merged

don't prefetch next item in loop context #1101

merged 2 commits into from Nov 8, 2019

Conversation

@davidism
Copy link
Member

davidism commented Nov 7, 2019

fixes #555

Previously, LoopContext prefetched the next item in the iterable, the internal iterator was advanced one further than the index. This caused issues with groupby or other iterators where items depend on the state of the iterator.

Now, the iterator will only advance early for the length, revindex, last and nextitem properties, using any other properties in the loop will not mess up groupby.

Unfortunately, this couldn't carry over to AsyncLoopContext directly. To look ahead in async, you need to await it.__anext__() rather than next(it). To await, you need to be in async def, but only __anext__ is async, not the properties, and we're not prefetching during iteration anymore. So this reimplements some properties to be async. This has the extra effect of making length, revindex, and revindex0 work for async iterables (they raised a TypeError before). The compiler must add await auto_await() around attribute access for these properties to resolve, previously it was only added for calls.

I feel like this implementation is cleaner overall, but I'm not sure of the implications for async performance.

davidism added 2 commits Nov 7, 2019
@davidism davidism marked this pull request as ready for review Nov 8, 2019
@davidism davidism added this to the 2.11.0 milestone Nov 8, 2019
@davidism davidism merged commit 540b260 into master Nov 8, 2019
10 checks passed
10 checks passed
Tests Build #20191108.1 succeeded
Details
Tests (Job Docs) Job Docs succeeded
Details
Tests (Job PyPy 3 Linux) Job PyPy 3 Linux succeeded
Details
Tests (Job Python 2.7 Linux) Job Python 2.7 Linux succeeded
Details
Tests (Job Python 2.7 Windows) Job Python 2.7 Windows succeeded
Details
Tests (Job Python 3.5 Linux) Job Python 3.5 Linux succeeded
Details
Tests (Job Python 3.6 Linux) Job Python 3.6 Linux succeeded
Details
Tests (Job Python 3.7 Linux) Job Python 3.7 Linux succeeded
Details
Tests (Job Python 3.7 Mac) Job Python 3.7 Mac succeeded
Details
Tests (Job Python 3.7 Windows) Job Python 3.7 Windows succeeded
Details
@davidism davidism deleted the refactor-loop-context branch Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

1 participant
You can’t perform that action at this time.