BUG: Improve Error Message for Multi-Char Sep + Quotes in Python Engine
State: closed

If there is a field counts mismatch, check whether a multi-char separator was used in conjunction with quotes.  Currently, that setup is not respected for the Python engine and can result in improper line breaks.  This PR appends that to the error message if that is a possible explanation.

Closes #13374.

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

Comments:
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14582?src=pr) is 85.22% (diff: 100%)
> Merging [#14582](https://codecov.io/gh/pandas-dev/pandas/pull/14582?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will increase coverage by **<.01%**

```diff
@@             master     #14582   diff @@
==========================================
  Files           143        143          
  Lines         50804      50807     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43292      43298     +6   
+ Misses         7512       7509     -3   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [75b606a...dda588b](https://codecov.io/gh/pandas-dev/pandas/compare/75b606abad51762b3faace1bbfa0a8d2241dc297...dda588b1d15df83f77f302282ec6f026a7e10591?src=pr)
Getting some weird segfaults that seem unrelated to this PR (also in my other one)...disabling testing for the time being.

@gfyoung yes, the segfaults are not your fault: https://github.com/pandas-dev/pandas/issues/14621

@jorisvandenbossche , @jreback : No more segfaulting, and everything looks green.  Ready to merge if there are no concerns.

I am not sure I am convinced this change is needed. @gfyoung Can you provide a bit more rationale in the top post and examples of how you to change your code or how the result changes? 

Reason that I am not fully convinced is that you are disallowing it for all cases where there are quotes in the csv file, while there is only a problem when the delimiter is present in the quoted fields.

@jorisvandenbossche , @jreback : Maybe the delimiter is not be present in the quoted fields, but how are we to check this without compromising performance in Python?  It is easier to insist upon passing in a parameter without raising an error than doing such a check.  This change is necessary then to patch the bug I showed in the related issue.

There is no real behaviour change, except for an error being raised depending on what separator is being passed in with the Python engine.  <strike>All tests that I modified perhaps could be left unchanged if I change some of the logic inside the parser but will need to check.</strike>

UPDATE: Not feasible because of what I provided <a href="https://github.com/pandas-dev/pandas/pull/14582#discussion_r88107407">above</a>.

@gfyoung It's quite possible your proposed change is the sensible thing to do, but I think you need to try to explain your changes and its consequences a bit better (in this case "Title is self-explanatory" is not really true .. :-))

As I understand it now, the consequence of your proposed change is that when somebody now does (on a file that does not necessarily contain quotes):

```
pd.read_csv(file, sep='\s+', engine='python')
```

this will raise an error now (so breaking code). To resolve this error, the user needs to add `quoting=csv.QUOTE_NONE` (as this is not the default). 
Is this correct?

In this case, for me this is a trade-off between having to provide an extra (unneeded) keyword in the simple case (no quotes in file) vs having a wrong error message in case of quotes + sep present in quotes (current behaviour that you are trying to fix).

@jorisvandenbossche : Fair enough.  Updated my <a href="https://github.com/pandas-dev/pandas/pull/14582#issue-187227924">description</a>.

@gfyoung Would it be possible to only raise the error if there are actually quotes in the data? (not sure how performant such a check would be) 
That will already solve the trade-off for a lot of usecases I think

@jorisvandenbossche : That's going to be tricky because we wrap all IO objects as a stream.  Unless you want to check every line when reading, I don't know of any other way to do it at the moment.  Thoughts?

@gfyoung why can't you just happily parse, then if the error happens, check the quoting paramater and raise the correct error?
@jreback : Because how can you tell if it's a legitimate error in the CSV file or incorrect parsing of quotation marks?  There isn't really a way to differentiate AFAICT.
@gfyoung 

>Because how can you tell if it's a legitimate error in the CSV file or incorrect parsing of quotation marks? There isn't really a way to differentiate AFAICT.

so maybe just say that in the error message. (IOW say you can pass ``quote.NONE`` to verify if no error.

I think *forcing* people to pass ``quote.NONE`` is pretty painful just to have a more general error message.
@jreback : Well, parsing regex separators is painful on our end.  You don't have to pass in `csv.None` if you're separator is only one character, so it shouldn't inconvenience people in most cases.
Hmm, we should try to make a decision here. Trying to summarize the options:

* Current situation: you get an error when having sep inside quoted field. From the error message this is not directly clear
  * Could we possibly do a check when such an error message occurs if there are quotes, and then add to the message: "possibly this is due to quotes that are ignored when having a multi-char sep"? (which is what @jreback proposed?) Or just adding it to the message without checking.
* PR: force people to use `quoting=csv.QUOTE_NONE` with the idea of making them aware of the possible problem with quotes.
  * The reason I am not really a fan: a) this also forces the user to use this when there is no potential problem (eg no quotes, or no sep chars inside the quotes), ànd b) this will still give you the same error message when you have sep chars inside quotes but *do* pass `csv.QUOTE_NONE` as the error message says to do
* Checking if there are quotes in the line, and only raising the error in that case
  * Disadvantages: possibly expensive performance wise + still forces users to use `quoting` when they have quotes without sep chars inside + makes implementation more complex for something that is probably not worth it.
* ...

@gfyoung @jreback Thoughts? 
Personally I would go for the first: leaving the current situation but improving the error message.
@jorisvandenbossche : You do have a point that the error message clarifies a lot what the issue is without necessarily inconveniencing the user.  Changed PR to follow the first suggestion.
lgtm.
@gfyoung thanks!