Update tokenizer to fix #8679 #8661
State: closed

Update tokenizer's handling of skipped lines.  
Fixes a problem with read_csv(delim_whitespace=True) and 
read_table(engine='c') when lines being skipped have trailing 
whitespace. 

closes #8679 
closes #8681 


Labels:
name: Bug
name: IO CSV
description: read_csv, to_csv

Comments:
pls add a release note in v0.15.2 (reference both issues)

cc @mdmueller pls have a look

Looks good to me--might as well leave the old test in as @jreback noted above though.

Thanks for the fix @selasley!

@selasley 
- pls add back old test
- add release note
- rebase / squash

@selasley ok, rebase again, you'll have a merge conflict (as you are including the whole of v0.15.2.txt again). You should be fine otherwise, repush and i'll have a look.

I added my comments to whatsnew/v0.15.2.txt from e8b3862dde, did a rebase/squash and pushed the result.  Let me know if I was supposed to do something different.

commit 3c16f33 contains a bug fix in tokenize_delim_customterm, general code cleanup and updates to various files to e8b3862d versions

you seem to be picking up other commits and then rebasing them in

what are you doing to rebase?

https://github.com/pydata/pandas/wiki/Using-Git

On Nov 10, 2014, at 3:37 AM, jreback notifications@github.com wrote:

> https://github.com/pydata/pandas/wiki/Using-Git

Thanks for the link.  I'll definitely read through it.  

I've been doing git rebase -i HEAD~2 then changing the second pick to squash and editing the commit comments.  Then I do a git push origin +trailing_spaces_fix

For the latest commit I copied several files from the latest master branch to my trailing_spaces_fix branch to make sure the tests still passed.  In retrospect that was probably not a good thing to do.=

@selasley can you rebase this.

and can you post a memory comparison of old vs new? (use a relatively small example)

I'll try again to rebase tonight.
I created a text file with 10000000 rows to skip, each containing 8 fields, followed by two rows each with 4 integers.  In ipython3
 %timeit -r 5 -n 5 df = pd.read_csv('bigfile.txt', header=None, delim_whitespace=True, skiprows=10000000)
5 loops, best of 5: 5.74 s per loop for new pandas
5 loops, best of 5: 10.3 s per loop for pandas 0.15.1 with numba and BottleNeck

According to the Allocations instrument with Xcode, reading the file allocated 4.67GB total with pandas 0.15.1 and 1.40GB with the new pandas.

With 100000 rows to skip the numbers are
5 loops, best of 5: 43.9 ms per loop, 29.23MB for new pandas
5 loops, best of 5: 88.0 ms per loop, 71.45MB for pandas 0.15.1

I found a problem with custom line terminators in 0.15.1 and in my version
pd.read_csv(StringIO('A B C_0 1 2_3 4 5'), lineterminator='_', sep=' ')
works as expected, but 
pd.read_csv(StringIO('A B C_0 1 2_3 4 5'.replace(' ', ',')), lineterminator='_', sep=r',+')
pd.read_csv(StringIO('A B C_0 1 2_3 4 5'), lineterminator='_', delim_whitespace=True)
pd.read_csv(StringIO('A  B  C_0  1  2_3  4  5'), lineterminator='_', sep=r'\ +')
all return an empty DataFrame.  I would like to fix this.

@selasley fine to fix (I think their is an open issue about line terminators - can't find it in s quick search)
pls put in separate commits (or do a separate pr)
the skip rows memory issue ok in here
but need to rebase

I think I rebased properly this time for the trailing spaces fix.  Thanks for your patience

rebase looks good!

https://github.com/pydata/pandas/wiki/Performance-Testing

see:
need to test for -r csv
eg the vbenches related to csv reading 
pls post results

I think it makes send to add s vbench for skiprows as well
don't make the example so long maybe
like them to take < less than say 500ms (or so)
for this type of bench - maybe 20k rows skip first 10k

I added a 20000 row, skiprows=10000 benchtest to io_bench.py

```
./test_perf.sh -b master -t HEAD -r csv

Invoked with :
--ncalls: 3
--repeats: 3


-------------------------------------------------------------------------------
Test name                                    | head[ms] | base[ms] |  ratio   |
-------------------------------------------------------------------------------
frame_to_csv_date_formatting                 |  21.4180 |  26.3120 |   0.8140 |
read_csv_skiprows                            |  16.4446 |  18.6180 |   0.8833 |
read_csv_precise_converter                   |   1.8516 |   1.9027 |   0.9732 |
frame_to_csv                                 | 159.6503 | 163.0680 |   0.9790 |
read_csv_thou_vb                             |  24.6903 |  25.0720 |   0.9848 |
frame_to_csv2                                | 149.5183 | 150.1943 |   0.9955 |
write_csv_standard                           |  55.2851 |  55.2817 |   1.0001 |
frame_to_csv_mixed                           | 967.0726 | 962.2053 |   1.0051 |
packers_write_csv                            | 1647.7884 | 1638.5774 |   1.0056 |
read_csv_infer_datetime_format_custom        |  13.7510 |  13.6327 |   1.0087 |
read_csv_infer_datetime_format_iso8601       |   2.2984 |   2.2717 |   1.0118 |
read_csv_infer_datetime_format_ymd           |   2.4959 |   2.4653 |   1.0124 |
read_csv_roundtrip_converter                 |   2.8143 |   2.7333 |   1.0296 |
packers_read_csv                             | 220.8546 | 213.4230 |   1.0348 |
read_csv_standard                            |  13.8597 |  13.3590 |   1.0375 |
read_csv_vb                                  |  26.3373 |  25.1070 |   1.0490 |
read_csv_default_converter                   |   2.0603 |   1.9317 |   1.0666 |
read_csv_comment2                            |  32.6854 |  30.3557 |   1.0767 |
-------------------------------------------------------------------------------
Test name                                    | head[ms] | base[ms] |  ratio   |
-------------------------------------------------------------------------------

Ratio < 1.0 means the target commit is faster then the baseline.
Seed used: 1234

Target [746afa2] : BUG CSV: fix problem with trailing whitespace, issues 8661, 8679
ENH CSV: improve memory footprint when skipping first N rows, issue 8681
Base   [cd93914] : BUG: Allow non-float values in get_yahoo_quotes.
Fixes issue: #5229
```

perf looks good (and vbench).

you need to rebase again. somehow picking up odd things.

`git rebase thisbranch -i origin/master` should do it

git rebase trailing_spaces_fix -i origin/master and git rebase -i trailing_spaces_fix origin/master gave fatal errors so I tried the rebase that seemed to work yesterday

@selasley pls rebase and push. is this working locally?

I am trying to follow the recipe on the wiki.  Starting from scratch
git clone https://github.com/selasley/pandas.git
git remote add upstream git://github.com/pydata/pandas
git fetch upstream
git rebase -i upstream/master
git diff usually shows that doc/source/whatsnew/v0.15.2.txt needs to be cleaned up.  I do that then
git rebase --continue
git push origin +trailing_spaces_fix
Right now git rebase -i says Successfully rebased and updated refs/heads/trailing_spaces_fix.
I did add some files to my fork and pushed before reading the wiki page.  Maybe that's what is causing problems.  Should I start from scratch and fork from the latest master branch?

`git push origin trailing_spaces_fix -f`

the `-f` is key

OK, I did a girl push with -f.  I mistakenly thought +trailing_spaces_fix was the same as --force from some stack overflow answer.  Now that I read the git push docs I see they are different.

https://travis-ci.org/pydata/pandas/jobs/41103767#L251

it's not cythonizing/compiling correctly

I misinterpreted the failure message on Travis.  It builds without errors on my mac with clang from Xcode 6.1 and Cython  0.21.1, but if I run nosetests in the pandas directory I get the ImportError: cannot import name 'hashtable' error. 
I'll work on getting it to build in Travis

looks good. minor comment. pls rebase / squash. ping when green.

I moved the comment to performance in whatsnew and rebased

replaced tm.assert_almost_equal(df.values, expected) with tm.assert_frame_equal(df, expected) where expected is now a dataframe.  I had to push a couple of times.  The latest travis build should succeed

thanks!

Although it works for "skiprows = integer" now, there is still problem with "skiprows = list of rows"
when the list does not start from row=0. 
such as read_table(file, skiprows = (1,2)) when all lines have trailing spaces