BUG: Corrects stopping logic when nrows argument is supplied (#7626)
State: closed

 - closes #7626 - Subsets of tabular files with different "shapes" will now load when a valid skiprows/nrows is given as an argument
 - [x] tests added / passed - Test case added to io/tests/parser/common.py. nosetests suite passed successfully.
 - [x] passes ``git diff upstream/master | flake8 --diff``

Conditions for error:
1) There are different "shapes" within a tabular data file, i.e. different numbers of columns.
2) A "narrower" set of columns is followed by a "wider" (more columns) one, and the narrower set is laid out such that the end of a 262144-byte block occurs within it.

Issue summary: 
The C engine for parsing files reads in 262144 bytes at a time. Previously, the "start_lines" variable in tokenizer.c/tokenize_bytes() was set incorrectly to the first line in that chunk, rather than the overall first row requested. This lead to incorrect logic on when to stop reading when nrows is supplied by the user. This always happened but only caused a crash when a wider set of columns followed in the file. In other cases, extra rows were read in but then harmlessly discarded.

This pull request always uses the first requested row for comparisons, so only nrows will be parsed  when supplied.

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

Comments:
can i create a small test that replicates the issue and fails on master (and passes here)?
Are you asking me to create the test or asking if it's OK for you to? If the latter, sure.

It's hard for me to say how large the file needs to be to create a crash but the bug can be reproduced at will using the file and row numbers jzwinck posted on the issue: https://gist.githubusercontent.com/jzwinck/838882fbc07f7c3a53992696ef364f66
@jeffcarey I am asking you to. The point is to have a regression test so future changes don't break this.
The issue / example is a last case resort. We much prefer to have a code based test (you can simply generate the appropriate data in memory; the size doesn't really matter). I just don't want to include external files like this. But if its impossible / hard, then you would need to include this test file.
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14747?src=pr) is 85.27% (diff: 100%)


> No coverage report found for **master** at 725453d.

> Powered by [Codecov](https://codecov.io?src=pr). Last update [725453d...e9c5bee](https://codecov.io/gh/pandas-dev/pandas/compare/725453deb2efae1b84d87131e785a42a22753659...e9c5bee32ebe81b28010adb8a994ec433d71a941?src=pr)
@jreback I've created a test case that does not rely on any external files, including the example. Where do you think the best place for it is in the test suite? I figured somewhere in the frame directory since it relates to creating a DataFrame, but I don't see anything about input. Let me know if you think it fits best into something that already exists or if it deserves to be in its own file.
https://github.com/pandas-dev/pandas/tree/master/pandas/io/tests/parser

has extensive tests for the parser

@jreback I've added a test and updated the comment here about the root cause. Please take a look.
@jreback Anything else needed here? I've fixed my test so that it complies with linting, all checks are green.
pls add a whatsnew note in 0.19.2.
Requested changes made, all green again. Please review.
@jeffcarey lgtm. ping on green.
@jreback All green
thanks @jeffcarey