bpo-12178: Fix escaping of escapechar in csv.writer()#13710
bpo-12178: Fix escaping of escapechar in csv.writer()#13710taleinat merged 1 commit intopython:masterfrom
Conversation
|
Since only the quote and escape chars should be escaped, perhaps add a test that other special characters, i.e. delimiters and line terminators, aren't? |
Lib/test/test_csv.py
Outdated
| (['a/', 1], '"a//",1', csv.QUOTE_MINIMAL), | ||
| (['a/', 1], 'a//,1', csv.QUOTE_NONE), | ||
| (['a/', 1], '"a//","1"', csv.QUOTE_ALL), | ||
| (['a/b', 1], '"a//b",1', csv.QUOTE_MINIMAL), |
There was a problem hiding this comment.
Are quotes needed here?
There was a problem hiding this comment.
@serhiy-storchaka, your question is unclear to me.
There was a problem hiding this comment.
Why the output is "a//b",1 instead of just a//b,1?
There was a problem hiding this comment.
Good question! I'm wondering the same thing now. Might just be a bug!
There was a problem hiding this comment.
It doesn't seem consistent with the docs:
Instructs
writerobjects to only quote those fields which contain special characters such as delimiter, quotechar or any of the characters in lineterminator.
There was a problem hiding this comment.
Yes, I agree there shouldn't be quotes here. They should be removed from the tests (and implemented correctly) before merging.
There was a problem hiding this comment.
This should be fixed now. Please take a look again.
|
I'm going to take a look at the PR this weekend. Sorry for my late response. |
|
@berkerpeksag, let me know if there's any way I can help this get into 3.8. |
|
Ping? We already missed 3.8, let's not miss 3.9! |
berkerpeksag
force-pushed
the
bpo-12178-csv
branch
from
4c46836 to
ea956f0
Compare
Co-Authored-By: Itay Elbirt <anotahacou@gmail.com>
berkerpeksag
force-pushed
the
bpo-12178-csv
branch
from
ea956f0 to
df92090
Compare
|
Ping, @berkerpeksag? |
|
I've already addressed review comments back in July. |
Ah, sorry, you're right, I missed that! |
|
Closing and re-opening to re-run the stalled Travis-CI check. |
|
@berkerpeksag, what do you think about whether this should be back-ported? While it's obviously a bug-fix, I'm conflicted since this could break code which relied on the existing behavior. |
I agree with you. I'm totally fine with either backporting to only 3.9 or no backport. |
|
3.9.0rc2 is out and only critical fixes should go into 3.9.0, and I wouldn't want this in 3.9.1 but not 3.9.0. So let's avoid back-porting. |
Co-authored-by: Itay Elbirt <anotahacou@gmail.com>
7 participants
https://bugs.python.org/issue12178