BUG: Respect usecols even with empty data
State: closed

closes #12493

 in which the `usecols` argument was not being respected for empty data.  This is because no filtering was applied when the first (and only) chunk was being read.


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

Comments:
Tests are passing.  Should be good to merge.

If some could cancel this build <a href="https://travis-ci.org/pydata/pandas/builds/113483314">#18023</a> (it's an old build), that would be great.  Thanks!

@jreback : Tests are passing once more.  Should be good to merge AFAICT.

@gfyoung you had to change a whole bunch of things to get your code to work (e.g. changing long established behavior in excel/html parsing), the `StopIteration` can now also be a `ValueError`. That is just crazy. My point is to make minimal changes that get the job done, use general methods where possible, and don't duplicate code.

@jreback : Simplified things a little by changing the error from `ValueError` to `StopIteration`.  I still kept some of the error messages from the C engine because I find them to be quite useful.

at a glance looks ok. still working on 0.18.0, so will get to next week.

:+1: will ping on all of them next week then.

don't continue to propogate the differnces between the c and python parser. Let's sync them up. The c-parser is almost certainly right (the cases are of course when a `ValueError/CParserError` is throw vs a `StopIteration`) which is for a completely different reason.

For each thing that is changing, you need to be VERY explict in the whatsnew note, showing an example of where a change is made.

What do you mean by "syncing" them up?  I'm a little stuck here because you said above that we should keep the changes as simple as possible.

EDIT: You answered my question above, so ignore this.

Actually, I just realized that once #12551 is landed, aligning `CParserError` errors in the C engine with `ValueError` errors in the Python engine will make a lot more sense since `CParserError` will then inherit from `ValueError`.

@jreback : I've attempted to sync up `CParserError` with `ValueError` on the Python side, but the problem is that it seems `StopIteration` is too ingrained into the code at the moment.  Converting those errors to `ValueError` suddenly causes breakages all over the place in `test_parsers.py` and other files like `test_html.py` or `test_excel.py`.

While I haven't made the commit here, I was able to locally change all the `CParserError`'s to  plain `ValueError`'s with no issues.  However, that is breaking backwards compatibility to a degree.  What do you think is the best way to proceed?

@jreback : It's not as simple as you make it or sound, or as I originally thought.  I wouldn't have made that comment if it was easy to do.  Changing that `StopIteration` alone breaks a good number of tests not just for `html.py` and `excel.py` but for `parsers.py` as well.  I will give it another shot today, but I think this change should be refrained for a separate PR if I can't manage.

Also, any comment on whether this PR _should_ include the conversion of `CParserError` to `ValueError`?

no, you should not change ANY `CParserErrors` to `ValueError`, it shouldn't be necessary. These are external errors. The issue ATM is that c-parser and python-parser return DIFFERENT errors. The `CParserError` (or `ValueError`) is correct and NOT `StopIteration`

I am running into an issue with the HTML parser in this test <a href="https://github.com/pydata/pandas/blob/master/pandas/io/tests/test_html.py#L629">here</a>.  It used to throw a `CParserError` but because I'm catching `ValueError` instead of `StopIteration`, the `CParserErrror` is swallowed, causing an empty list of tables to be returned as expected with no exceptions being thrown.  Thoughts?

@gfyoung I would just change that test, a `ValueError` should be thrown.

A `ValueError` will never be thrown because `html.py` catches them instead of `StopIteration`, so the `ValueError` never trickles up to the surface.

no, what I mean is that INSIDE html.py you will need to catch the `ValueError` and re-raise as (or let it bubble up). These should all have exactly the same semantics. I think they got somewhat muddled because of the differences between c/python parser.

How can I tell the difference between a deliberately raised `ValueError` and ones that are just caught and can be ignored?  Is checking the error message a valid option?

EDIT: let me rephrase, can I check the error message to see which ones I can ignore and which ones should actually raise an error?

Do you _need_ to catch them? I don't really recall in that code.

generally we really really try to avoid doing that kind of checking.

Yes.  The code deliberately catches `StopIteration` because it means there is an empty table (there is even a comment next to the `catch` statement right now to explain why).  The same is true for `excel.py`.

so is the issue that you can't tell the difference between an empty table or one that simply could not be read? aren't these functionaily equivalent from html/excel perspective anyhow?

Yes, when I change it to `ValueError`, I can no longer tell the difference between malformed and empty.  I am not entirely sure at the moment what is best.  From a user perspective, I would think raising on a malformed table makes sense, but an empty table (especially in an HTML file) isn't necessarily something that I would "punish" the user for having though.

Let's go ahead and change it and just catch/ignore the `ValueError`. This is ONLY because we are using the python parser, which doesn't make much sense. These should be the same.

Sounds good.  I'll also put in API changes into the `whatsnew` for both `html.py` and `excel.py`.

I didn't follow the discussion in this PR (so please bear my ignorance), but I just read the whatsnew note, and I have to say that it's not fully clear to me:

> The HTML and Excel parsers no longer differentiate between empty data and parsing errors and will return an empty tables if either situation occurs

Does this exactly mean what is said there? So that if you have for some reason an error parsing a file with data, you silently get back an empty dataframe instead of being notified of this error? 
How is that an improvement? 

@jorisvandenbossche : Yes, it does mean exactly what it says.  If you read the conversation above, that could give you a better context as to why that decision was made.

@jreback : Travis has given the green light with the changes to `html.py` and `excel.py` so this is ready to land if there is nothing else.

@gfyoung so the way to actually do this properly and not swallow errors
is the make a EmptyParsingError(ValueError) or something like this
and raise it (and catch at the appropriate place)

then ValueErrors will bubble up and you can catch empty partings if needed

@jreback , @jorisvandenbossche : I think I got it this time around, and Travis gives the green light.  If there is nothing else, this is ready to merge.

@jreback : I refactored the `EmptyDataError` into `parser.pyx` so that the C engine can use it, and Travis is happy with that change.  So I guess it really depends now on whether we should split the PR or merge it as is.