You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
csv.reader/writer to raise exception if mode is binary or newline is not '' #55163
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=Noneclosed_at=<Date2021-10-20.23:10:57.030>created_at=<Date2011-01-20.10:32:25.324>labels= ['expert-2to3', 'type-bug', 'library']
title="csv.reader/writer to raise exception if mode is binary or newline is not ''"updated_at=<Date2021-10-20.23:10:57.029>user='https://github.com/regebro'
In Python 2 the file used for csv.writer() should be opened in binary mode, while in Python 3 is should be opened in text mode but with newlines set to ''.
This change is neither warned for by python -3, nor is there a fixer for it (and making a fixer would be tricky), thus it provides a surprising API change.
I think that csv.writer() should warn or even fail if the file is opened in binary mode under Python 3. Failing is a god option, as a binary file is likely to be a port from Python 2, and you are likely to get the less useful message "must be bytes or buffer, not str". Even if you understand that message, you will then probably just change the file mode from binary to text, but you will not add the lineendings='' parameter, and thusly you might cause subtle error on windows.
Failing when passed a bytesIO object seems reasonable.
I question the bit about newlines though. The doc does not specify that newlines='' is needed on output. While is says it is needed for input, why? Why is a mix of '\n', '\r\n', and '\r' better than always '\n'?
It is not clear to me that the information is available to csv anyway.
Newline='' is indeed needed. It preserves the newlines so that the csv module can correctly parse them according to the weird csv quoting roles. And for output, the fact that it isn't documented there is a an issue that was only noticed recently.
sjmachin commented on Jan 23, 2011
sjmachinmannequin
Mannequin
More actions
I don't understand "Changing csv api is a feature request that could only happen in 3.3". This is NOT a request for an API change. Lennert's point is that an API change was made in 3.0 as compared with 2.6 but there is no fixer in 2to3. What is requested is for csv.reader/writer to give more meaningful error messages for valid 2.x code that has been put through fixer-less 2to3.
The name of the arg is "newline". "newlines" is an attribute that stores what was actually found in "universal newlines" mode.
newline='' is needed on input for the same reason that binary mode is required in 2.x: \r and \n may quite validly appear in data, inside a quoted field, and must not be treated as part of a row separator.
newline='' is needed on output for the same reason that binary mode is required in 2.x: any \n in the data and any \n in the caller's chosen "line" terminator must be preserved from being changed to os.linesep (e.g. \r\n).
"newline" is not available as an attribute of the _io.TextIOWrapper object created by open('xxx.csv', 'w', newline=''); is exposing this possible?
regebro commented on Jan 23, 2011
regebromannequin
MannequinAuthor
More actions
In the worst case, not checking for newline='' is not a big problem, as anyone moving from Python 2 to Python 3 will open the file in binary mode. That error message could tell the user to use binary mode newlines=''.
Using textmode and newlines is only likely to happen with people writing new code for Python 3 and not reading the docs, which is a different problem. :-)
The API change would be generating an error if newline='' wasn't specified. Amplifying the bytes-case error message would be fine, though. On the other hand, we are in RC phase, and I'm not at all sure this is important enough to go in to an RC. On the gripping hand it is also trivial enough that Georg might approve it.
Looking at the csv.rst file I see this statement early in the py3k
docs:
If csvfile is a file object, it should be opened with newline=''.
There is also a footnote about the consequences of leaving it out:
.. [#] If newline='' is not specified, newlines embedded inside quoted fields
will not be interpreted correctly. It should always be safe to specify newline='', since the csv module does its own universal newline handling
on input.
Finally, the examples all use "newline=''". I see two things to change
in the docs:
Replace "should" with "must" in the first quoted sentence above.
Add that sentence to the documentation for the csv.writer()
function.
Skip, the docs bug is bpo-7198. This is the meaningful-exception bug.
sjmachin commented on Mar 19, 2011
sjmachinmannequin
Mannequin
More actions
The doc patch proposed by Skip on 2001-01-24 for this bug has NOT been reviewed, let alone applied. Sibling bug bpo-7198 has been closed in error. Somebody please help.
Since this is not a doc issue, doc people would not especially see it. That aside...
What is *your* review. Does it satisfy you?
Answer on bpo-7198 if you want.
And please be a bit patient as people are learning the new hg system.
sjmachin commented on Mar 19, 2011
sjmachinmannequin
Mannequin
More actions
Terry, I have already made the point """the docs bug is bpo-7198. This is the meaningful-exception bug."""
My review is """changing 'should' to 'must' is not very useful without a consistent interpretation of what those two words mean and without any enforcement of use of newline=''.
I was patient enough to wait 2 months for a review of my "doc patch" on bpo-7198.
My issues are that the 3.2 docs have NOT been changed (have a look at the csv.writer paragraph: do you see the word "newline" anywhere??), bpo-7198 has been closed without any action, and BOTH of these two issues (which have in effect been lurking about since Python 3.0.0alpha) appear to have been abandoned.
BreamoreBoy commented on Feb 24, 2015
BreamoreBoymannequin
Mannequin
More actions
I've changed this issue to reflect what I think it should be saying.
changed the title [-]No warning for csv.writer API change[/-][+]csv.reader/writer to raise exception if mode is binary or newline is not ''[/+]on Feb 24, 2015
I suspect it may not be practical to check the newline translation mode of a TextIOWrapper or StringIO stream, and I don’t think newline translation is even required in general for text stream classes. Beware that the “newlines” attribute isn’t going to help; see bpo-8895.
Activity
regebro commentedon Jan 20, 2011
In Python 2 the file used for csv.writer() should be opened in binary mode, while in Python 3 is should be opened in text mode but with newlines set to ''.
This change is neither warned for by python -3, nor is there a fixer for it (and making a fixer would be tricky), thus it provides a surprising API change.
I think that csv.writer() should warn or even fail if the file is opened in binary mode under Python 3. Failing is a god option, as a binary file is likely to be a port from Python 2, and you are likely to get the less useful message "must be bytes or buffer, not str". Even if you understand that message, you will then probably just change the file mode from binary to text, but you will not add the lineendings='' parameter, and thusly you might cause subtle error on windows.
sjmachin commentedon Jan 20, 2011
I believe that both csv.reader and csv.writer should fail with a meaningful message if mode is binary or newline is not ''
terryjreedy commentedon Jan 22, 2011
Failing when passed a bytesIO object seems reasonable.
I question the bit about newlines though. The doc does not specify that newlines='' is needed on output. While is says it is needed for input, why? Why is a mix of '\n', '\r\n', and '\r' better than always '\n'?
It is not clear to me that the information is available to csv anyway.
terryjreedy commentedon Jan 22, 2011
Changing csv api is a feature request that could only happen in 3.3.
bitdancer commentedon Jan 22, 2011
Newline='' is indeed needed. It preserves the newlines so that the csv module can correctly parse them according to the weird csv quoting roles. And for output, the fact that it isn't documented there is a an issue that was only noticed recently.
sjmachin commentedon Jan 23, 2011
I don't understand "Changing csv api is a feature request that could only happen in 3.3". This is NOT a request for an API change. Lennert's point is that an API change was made in 3.0 as compared with 2.6 but there is no fixer in 2to3. What is requested is for csv.reader/writer to give more meaningful error messages for valid 2.x code that has been put through fixer-less 2to3.
The name of the arg is "newline". "newlines" is an attribute that stores what was actually found in "universal newlines" mode.
newline='' is needed on input for the same reason that binary mode is required in 2.x: \r and \n may quite validly appear in data, inside a quoted field, and must not be treated as part of a row separator.
newline='' is needed on output for the same reason that binary mode is required in 2.x: any \n in the data and any \n in the caller's chosen "line" terminator must be preserved from being changed to os.linesep (e.g. \r\n).
"newline" is not available as an attribute of the _io.TextIOWrapper object created by open('xxx.csv', 'w', newline=''); is exposing this possible?
regebro commentedon Jan 23, 2011
In the worst case, not checking for newline='' is not a big problem, as anyone moving from Python 2 to Python 3 will open the file in binary mode. That error message could tell the user to use binary mode newlines=''.
Using textmode and newlines is only likely to happen with people writing new code for Python 3 and not reading the docs, which is a different problem. :-)
bitdancer commentedon Jan 23, 2011
The API change would be generating an error if newline='' wasn't specified. Amplifying the bytes-case error message would be fine, though. On the other hand, we are in RC phase, and I'm not at all sure this is important enough to go in to an RC. On the gripping hand it is also trivial enough that Georg might approve it.
birkenfeld commentedon Jan 23, 2011
Can we have a concrete proposal in the form of a patch, please?
smontanaro commentedon Jan 23, 2011
Looking at the csv.rst file I see this statement early in the py3k
docs:
If csvfile is a file object, it should be opened with
newline=''.There is also a footnote about the consequences of leaving it out:
.. [#] If
newline=''is not specified, newlines embedded inside quoted fieldswill not be interpreted correctly. It should always be safe to specify
newline='', since the csv module does its own universal newline handlingon input.
Finally, the examples all use "newline=''". I see two things to change
in the docs:
function.
smontanaro commentedon Jan 23, 2011
My suggestion attached.
sjmachin commentedon Jan 23, 2011
Skip, the docs bug is bpo-7198. This is the meaningful-exception bug.
sjmachin commentedon Mar 19, 2011
The doc patch proposed by Skip on 2001-01-24 for this bug has NOT been reviewed, let alone applied. Sibling bug bpo-7198 has been closed in error. Somebody please help.
terryjreedy commentedon Mar 19, 2011
Since this is not a doc issue, doc people would not especially see it. That aside...
What is *your* review. Does it satisfy you?
Answer on bpo-7198 if you want.
And please be a bit patient as people are learning the new hg system.
sjmachin commentedon Mar 19, 2011
Terry, I have already made the point """the docs bug is bpo-7198. This is the meaningful-exception bug."""
My review is """changing 'should' to 'must' is not very useful without a consistent interpretation of what those two words mean and without any enforcement of use of newline=''.
I was patient enough to wait 2 months for a review of my "doc patch" on bpo-7198.
My issues are that the 3.2 docs have NOT been changed (have a look at the csv.writer paragraph: do you see the word "newline" anywhere??), bpo-7198 has been closed without any action, and BOTH of these two issues (which have in effect been lurking about since Python 3.0.0alpha) appear to have been abandoned.
BreamoreBoy commentedon Feb 24, 2015
I've changed this issue to reflect what I think it should be saying.
[-]No warning for csv.writer API change[/-][+]csv.reader/writer to raise exception if mode is binary or newline is not ''[/+]vadmium commentedon Feb 24, 2015
I suspect it may not be practical to check the newline translation mode of a TextIOWrapper or StringIO stream, and I don’t think newline translation is even required in general for text stream classes. Beware that the “newlines” attribute isn’t going to help; see bpo-8895.