csv sniffer does not recognize quotes at the end of line

[MartinBudaj]

BPO	10515
Nosy	@ned-deily, @bitdancer
Superseder	bpo-30157: csv.Sniffer.sniff() regex error
Files	fix.tar.gz test.py: file 1 of 3 from fix.tar.gz p1.patch: file 2 of 3 from fix.tar.gz p2.patch: file 3 of 3 from tix.tar.gz test_csv.patch csv_delimiter_tests.patch csv_delimiter_tests.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-03-01.20:03:14.670>
created_at = <Date 2010-11-23.17:55:40.955>
labels = ['type-bug', 'library']
title = 'csv sniffer does not recognize quotes at the end of line'
updated_at = <Date 2022-03-01.20:03:14.670>
user = 'https://bugs.python.org/MartinBudaj'

bugs.python.org fields:

activity = <Date 2022-03-01.20:03:14.670>
actor = 'iritkatriel'
assignee = 'none'
closed = True
closed_date = <Date 2022-03-01.20:03:14.670>
closer = 'iritkatriel'
components = ['Library (Lib)']
creation = <Date 2010-11-23.17:55:40.955>
creator = 'Martin.Budaj'
dependencies = []
files = ['19799', '19800', '19802', '19803', '19944', '20006', '20111']
hgrepos = []
issue_num = 10515
keywords = ['patch']
message_count = 16.0
messages = ['122227', '122230', '122312', '122316', '123100', '123406', '123772', '123773', '123787', '123791', '123949', '124354', '124359', '124361', '124362', '236542']
nosy_count = 4.0
nosy_names = ['andrewmcnamara', 'ned.deily', 'r.david.murray', 'Martin.Budaj']
pr_nums = []
priority = 'normal'
resolution = 'duplicate'
stage = 'resolved'
status = 'closed'
superseder = '30157'
type = 'behavior'
url = 'https://bugs.python.org/issue10515'
versions = ['Python 3.1', 'Python 2.7', 'Python 3.2']

[MartinBudaj]

The method Sniffer._guess_quote_and_delimiter() in the module csv.py contains a bug in a regexp which checks for quotes around the last item of the line (example: a,b,"c,d"\n).

the pattern
'(?P<delim>>[^\\w\\n"\\'])(?P<space> ?)(?P<quote>["\']).*?(?P=quote)(?:$|\n)'
used in the regex should be changed to
'(?P<delim>[^\\w\\n"\\'])(?P<space> ?)(?P<quote>["\']).*?(?P=quote)(?:$|\n)'

file csv.py: line 212 in python 2.6, line 216 in 2.7, line 207 in 3.1

[ned-deily]

Thanks for the report. It would be helpful if you could supply a patch including a unit test for this against 3.2 and/or 2.7. Note only security issues are accepted for 2.6.

[MartinBudaj]

Units test and two patches for 2.7 are included.

p1.patch fixes testEnd case reported yesterday

After running unittest it seems that also other case is broken (testAl -- if there is just one data item on the line, enclosed in quotes). Patch p2 fixes it, but should be checked for side-effects.

[ned-deily]

(Thanks. I've unpacked and uploaded the three files from your tar file.)

[bitdancer]

Are you sure testA1 is correct? It seems to me that in that case the sniffer can indeed not determine the delimiter, but I don't really understand the guessing algorithm. The existing behavior on unquoted strings is...interesting :)

Also if you are willing to take the time it would be helpful to have the tests as a patch against test_csv.

[MartinBudaj]

I'm not sure about what the intended behavior for testAl should be, however I think that the file should be recognized as having one column of data and no delimiter (there is a test for this case in csv.py) and not raise an exception.

I attach patch for test_csv.py tests. It includes two more tests which currently fail (both contain \r\n as a newline)

[bitdancer]

What do you mean by "there is a test for this case in csv.py"? If I run sniffer against "abcde\ndefgh\n" I get a delimiter of 'e'. If I run it against 'a\nb\n', I get the could not determine delimiter error.

Attached is a reformulated test case that lets us see the errors individually and easily add more. With this set of 10 tests, two pass without your patches (as you knew; those are your tests), and we have 4 failures and 4 errors. With p1 applied we have 4 failures and 3 errors. With p2 applied we have 5 failures.

As I said, I'm not at all sure what the results *should* be for various of these cases. Certainly the long standing existing behavior seems to be to return one of the characters in an unquoted multicharacter sequence.

That said, it seems like your patch p1 is good, but as you discovered not sufficient.

[bitdancer]

Forgot to attach the patch.

[smontanaro]

From the comment in the test_csv.py:

+ # XXX: I don't know what the correct behavior should be for these.
+ # Currently the first one raises an error that the delimiter can't
+ # be determined while the second one returns '\r'. The second
+ # is obviously.
+ ('"a,b,c,d"\ne', ''),
+ ('"a,b,c,d"\r\ne', ''),

Obviously what? My guess would be "wrong". In the absence of any other
information \r\n has to be treated as a line separator. It shouldn't be
considered as two separate characters, even in such a devoid-of-clues test
case.

Is the empty string a valid delimiter? I've never tried it and it's been a
long time since I looked at any of the code. I do use single-column CSV
files from time-to-time though. I don't think a ',' would be a completely
unreasonable fallback scenario either.

Skip

[bitdancer]

Yeah, obviously wrong. I forgot to finish editing the comment.

I think a fallback of ',' makes more sense than ''. What would a delimiter of nothing mean? I don't think the unquoted case can be changed for backward compatibility reasons, so those tests I added should presumably be changed to confirm the existing behavior (with an appropriate comment).

[MartinBudaj]

What do you mean by "there is a test for this case in csv.py"?

I meant test in regex on line 217 in python 2.7 and the following code (line 258ff):

# there is *no* delimiter, it's a single column of quoted data
delim = ''
skipinitialspace = 0

However, it is intended to detect just lines starting and ending with quotes.

[smontanaro]

I get two failures with David's latest patch. Abstracting from
a lightly modified patch:

sample was: 'a,b,"c,d"\r\ne,f,g', got: '', expected: ','
sample was: '"a,b,c,d"\r\ne', got: '\r', expected: ''

In both cases I think the expected delimiter should be comma.
The code or regular expressions need to be fixed for the first
failure. As I indicated earlier \r\n should never be split.
Also, for a single column CSV file I think the expected/default
delimiter should be a comma.

That said, does anyone think I should change the expected delimiter
for the first two tests at this point for 2.7? The inputs and
expected delimiters are

('abcde', 'e'),
('a', 'e'),

Both seem wrong to me, but I understand that the behavior of 2.7
probably shouldn't change to accommodate fixing broken test cases.

I'm also a bit concerned about this construct:

locals()['some name'] = f

I thought locals() wasn't guaranteed to actually refer to a
persistent dictionary. That is, it's fine if locals() builds
its dictionary on-the-fly then discards it. Do we know for
a fact that when called during a class definition that locals()
returns the class's dictionary? (This might only matter if
other dialects of Python try to reuse test_csv.py.)