ENH: Implemented lazy iteration #20796
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like the 2.7 failures are relevant. This would need a release note. How's the performance when actually iterating? e.g. df = pd.DataFrame({"A": np.arange(100000)})
list(iter(df.itertuples())) |
Yea, I think I just managed to fix those. I had to import
Where does this go? Any instructions anywhere?
Memory wise: great. It just has to have one row at a time in memory. CPU wise: it looks like around 2-3x slower than Oh, and of course. This version immediately starts returning values while before you had to wait for everything to finish. So latency is much lower, while overall time is around 2-3x. |
Codecov Report
@@ Coverage Diff @@
## master #20796 +/- ##
==========================================
+ Coverage 92.3% 92.3% +<.01%
==========================================
Files 163 163
Lines 51943 51947 +4
==========================================
+ Hits 47946 47951 +5
+ Misses 3997 3996 -1
Continue to review full report at Codecov.
|
|
Correction. I was testing before just iterating directly on I used this to test both how long it takes to create a list of all tuples, how long it takes to get the first tuple, and how long it takes to get only simple tuples. import numpy as np
import pandas as pd
import time
print(pd.__path__)
def perf(f):
start = time.perf_counter()
f()
end = time.perf_counter()
return end - start
def a():
list(iter(df.itertuples()))
def b():
next(iter(df.itertuples()))
def c():
list(iter(df.itertuples(index=False, name=None)))
df = pd.DataFrame({"A": np.arange(10000000)})
print(sum(perf(a) for i in range(10)) / 10)
print(sum(perf(b) for i in range(10)) / 10)
print(sum(perf(c) for i in range(10)) / 10)Old version: New version: |
|
It is interesting. It seems this even makes the regular (Series version) slightly faster, while simple-tuple version is slower a bit. I could repeat these results even after trying a bit more. |
|
If you want to measure performance, we also have an asv performance benchmark for this method here: pandas/asv_bench/benchmarks/frame_methods.py Lines 106 to 108 in add3fbf Here's a guide on how to run the asv benchmark to evaluate performance changes. |
mroeschke
reviewed
Apr 23, 2018
pandas/core/frame.py
Outdated
| fields.append("Index") | ||
|
|
||
| # use integer indexing because of possible duplicate column names | ||
| arrays.extend(self.iloc[:, k] for k in range(len(self.columns))) | ||
| iterators.extend(self.iloc[:, k] for k in range(len(self.columns))) |
There was a problem hiding this comment.
I believe iterators can also be evaluated lazily by using itertools.chain. Something along the lines of:
iterators = itertools.chain([self.index], (self.iloc[:, k] for k in range(len(self.columns)))
There was a problem hiding this comment.
Yea, but I do not think this is necessary, because those are per-column and have to be done immediately afterwards even for the first row. So you move forcing execution from one line in a function to another line in the same function. So later on we do zip(*iterators) which forces iterators to be made into a list. If we would manually zip things, then we could go without that. But passing *iterators forces the list. It is also better that any exception evaluating columns is thrown here and not later inside that other try/catch.
So I do not think this is necessary.
|
Tests are failing for some unrelated reason. |
|
I ran benchmarks, but changes are all over the place. Both positive and negative. For same kind of methods (like rolling) some go up and down. I am not sure how stable are those benchmarks. I have to leave it for few hours to run and could not assure complete idleness of the computer. Also, absolute times are just few ms for many of them. I think this is hard to measure well. |
What you can do if you suspect a given result is a fluke is to rerun only that benchmarks file, as in:
this should take just a couple of minutes (assuming you just ran another asv test on the same commits), during which you can probably leave your computer idle. |
jreback
requested changes
May 8, 2018
mitar
force-pushed
the
iterators
branch
3 times, most recently
from
ad99292 to
868f8db
Compare
|
I think I updated everything requested in the code. I am now running benchmarks once more on an idle machine. |
|
There already seems to be an |
|
I added few more asv tests. |
|
Updated benchmarks: |
|
Just iteration, ran again: |
jreback
requested changes
May 29, 2018
doc/source/whatsnew/v0.23.1.txt
Outdated
| @@ -16,6 +16,8 @@ New features | |||
| ~~~~~~~~~~~~ | |||
|
|
|||
| - :meth:`Index.droplevel` is now implemented also for flat indexes, for compatibility with MultiIndex (:issue:`21115`) | |||
| - Iterating over a :class:`Series` and using :meth:`DataFrame.itertuples` now create iterators without internally | |||
There was a problem hiding this comment.
move to 0.24.0 performance section
|
@mitar did you run this backwards? those times have increased |
I ran I am not sure how to determine better what is happening. |
mitar
commented
May 29, 2018
Fixes GH20783.
…1d_sparse benchmark" This reverts commit e2f892a.
|
@TomAugspurger - Thanks! I've rebased to your commit, I suppose we should be good now. |
|
I've rerun the benchmark, I assume a lot of changes are due to the fix. |
|
can you merge master |
|
can you add whatsnew note in perf section |
|
why are these 3 increasing? |
|
Previous implementation would load the entire dataset into Python memory and the iterate over it. Now we iterate over the underlying array which reduces read speed. I think this is in line with the previous discussion, we are trying to optimize for (initialization) memory use and initialization time rather than total execution time. The to_list benchmarks run |
|
jreback
approved these changes
Dec 25, 2018
|
Thanks @jreback, @mitar and @TomAugspurger ! :) |
Pingviinituutti
pushed a commit
to Pingviinituutti/pandas
that referenced
this pull request
Feb 28, 2019
Pingviinituutti
pushed a commit
to Pingviinituutti/pandas
that referenced
this pull request
Feb 28, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes GH20783.
git diff upstream/master -u -- "*.py" | flake8 --diff