BUG: `line terminator` and '\n to \r\n' problem in Windows(Issue #20353)
State: closed

- [ ] closes #20353
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: IO CSV
description: read_csv, to_csv
name: Windows
description: Windows OS

Comments:
Hello @deflatSOCO! Thanks for updating the PR.

 - There are no PEP8 issues in the file [`pandas/core/generic.py`](https://github.com/deflatSOCO/pandas/blob/23ce5c6cf5fd649a19692a9bda0fbc844534127e/pandas/core/generic.py) !

 - There are no PEP8 issues in the file [`pandas/io/common.py`](https://github.com/deflatSOCO/pandas/blob/23ce5c6cf5fd649a19692a9bda0fbc844534127e/pandas/io/common.py) !

 - There are no PEP8 issues in the file [`pandas/io/formats/csvs.py`](https://github.com/deflatSOCO/pandas/blob/23ce5c6cf5fd649a19692a9bda0fbc844534127e/pandas/io/formats/csvs.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_to_csv.py`](https://github.com/deflatSOCO/pandas/blob/23ce5c6cf5fd649a19692a9bda0fbc844534127e/pandas/tests/frame/test_to_csv.py) !

 - There are no PEP8 issues in the file [`pandas/tests/io/formats/test_to_csv.py`](https://github.com/deflatSOCO/pandas/blob/23ce5c6cf5fd649a19692a9bda0fbc844534127e/pandas/tests/io/formats/test_to_csv.py) !

 - There are no PEP8 issues in the file [`pandas/tests/io/test_html.py`](https://github.com/deflatSOCO/pandas/blob/23ce5c6cf5fd649a19692a9bda0fbc844534127e/pandas/tests/io/test_html.py) !

 - There are no PEP8 issues in the file [`pandas/tests/util/test_testing.py`](https://github.com/deflatSOCO/pandas/blob/23ce5c6cf5fd649a19692a9bda0fbc844534127e/pandas/tests/util/test_testing.py) !

 - There are no PEP8 issues in the file [`pandas/util/testing.py`](https://github.com/deflatSOCO/pandas/blob/23ce5c6cf5fd649a19692a9bda0fbc844534127e/pandas/util/testing.py) !

 - Complete extra results for this file :

> file_to_check.py:2521:-258: W605 invalid escape sequence '\('
file_to_check.py:2521:-255: W605 invalid escape sequence '\)'
---



##### Comment last updated on October 19, 2018 at 05:43 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21406?src=pr&el=h1) Report
> Merging [#21406](https://codecov.io/gh/pandas-dev/pandas/pull/21406?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/ecc5cbc64ea44e010fa7fe9d25e3064926594b09?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `88.88%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/21406/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/21406?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #21406      +/-   ##
==========================================
- Coverage   92.19%   92.19%   -0.01%     
==========================================
  Files         169      169              
  Lines       50950    50956       +6     
==========================================
+ Hits        46975    46980       +5     
- Misses       3975     3976       +1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.61% <88.88%> (-0.01%)` | :arrow_down: |
| #single | `42.27% <33.33%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/21406?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/21406/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `96.65% <ø> (ø)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/21406/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `86.5% <100%> (+0.05%)` | :arrow_up: |
| [pandas/io/formats/csvs.py](https://codecov.io/gh/pandas-dev/pandas/pull/21406/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvY3N2cy5weQ==) | `98.22% <100%> (+0.01%)` | :arrow_up: |
| [pandas/io/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/21406/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2NvbW1vbi5weQ==) | `70.76% <66.66%> (-0.28%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21406?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21406?src=pr&el=footer). Last update [ecc5cbc...23ce5c6](https://codecov.io/gh/pandas-dev/pandas/pull/21406?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Willing to be convinced otherwise but I don't think the default should change to `\n` on Windows.  It's an annoying convention, but convention all the same.
@chris-b1 : The issue more relates to an inconsistency, as <a href="https://github.com/pandas-dev/pandas/issues/20353#issuecomment-392301505">here</a>.
Right, and I'm good with fixing the original bug (`\r\n` terminator when `\n` explicitly passed), but this PR would, if I'm understanding correctly, also change the default to `\n` when no line terminator is passed.
> but this PR would, if I'm understanding correctly, also change the default to \n when no line terminator is passed.

@chris-b1 : Ah, I see what you mean.  That is indeed correct based on the tests.

@deflatSOCO : Could you change it so that the default remains `\r\n` unless otherwise specified?
@chris-b1 @gfyoung  Please let me convince.
[The current document](http://pandas.pydata.org/pandas-docs/version/0.23/generated/pandas.DataFrame.to_csv.html) says the default value of line_terminator in `to_csv` is **'\n'** no matter what OS is, so I just followed that in this PR.
I suppose that you mean that should be changed to '\r\n' only in Windows.

If so, the expected output undet the Windows environment should be like this. Is that right?
* code
```
data = pd.DataFrame({
    "integer":[1,2,3],
    "string_lf":["abc","d\nef","gh\r\ni"]
})
data.to_csv('test.txt', index=False)
print(pd.read_csv('test.csv'))
data.to_csv('test.csv', index=False)
print(pd.read_csv('test.csv'))
with open("test.csv", mode='rb') as f:
    print(f.read())
```
* current output(v0.23)
```
   integer  string_lf
0        1        abc
1        2    d\r\nef
2        3  gh\r\r\ni
b'integer,string_lf\r\n1,abc\r\n2,"d\r\nef"\r\n3,"gh\r\r\ni"\r\n'
```
* current output(this PR)
```
   integer  string_lf
0        1        abc
1        2      d\nef
2        3    gh\r\ni
b'integer,string_lf\n1,abc\n2,"d\nef"\n3,"gh\ni"\n'
```
* expected output
```
   integer  string_lf
0        1        abc
1        2      d\nef
2        3    gh\r\ni
b'integer,string_lf\r\n1,abc\r\n2,"d\nef"\r\n3,"gh\ni"\r\n'
```
> @chris-b1 @gfyoung Please let me convince.
The current document says the default value of line_terminator in to_csv is '\n' no matter what OS is, so I just followed that in this PR.

@chris-b1 : Thoughts?  IMO @deflatSOCO has a point here.
@jreback @chris-b1 : Could you take a look at this?
I know the current docs say contrary, but my vote is that we continue defaulting to the platform default line terminator (`\r\n` on Windows) and updating the docs as such.

It preserves backwards compat - so if someone was relying on the `\r` being there, their code won't break, and is consistent with other programs on Windows that save csvs, including Excel, and python's csv writer.

```python
In [1]: import csv

In [3]: with open('tmp.csv', 'wt', newline='') as f:
   ...:     writer = csv.writer(f)
   ...:     writer.writerow(['a', 'b'])
   ...:     writer.writerow([1, 2])
   ...:     writer.writerow([3, 4])

In [4]: open('tmp.csv', 'rb').read()
Out[4]: b'a,b\r\n1,2\r\n3,4\r\n'
```

(again fully in support of merging this with bugfix for case with explicit ending)
can u rebase
> I know the current docs say contrary, but my vote is that we continue defaulting to the platform default line terminator (\r\n on Windows) and updating the docs as such.

Fair enough.  @deflatSOCO : Can you update?

(If you can't, I can pick this up)
Marking for `0.24.0`, as I can bring this to the finish line if need be.
@deflatSOCO : Could you address the merge conflicts here?

@TomAugspurger @jreback : Can you have a look again at this?
@TomAugspurger @jreback : Can you have a look again at this?
@deflatSOCO can you update the  whatsnew as discussed above, and rebase on master.
@jreback I applied in commit e4badc4, and rebased. Please check them.
just a couple of minor comments / doc-string things. @gfyoung ping when happy.
@gfyoung you ok with this, can you rebase this? or @deflatSOCO 
@jreback : Can do, given that no reply how has come since the latest comment in > 1 week.
Alas, there were too many merge conflicts with `master` that I had to nuke the old commits and replace them with a squashed version.
@jreback : Addressed all comments, rebased, and all is green!  PTAL.
thanks @deflatSOCO and @gfyoung