BUG: Fix default encoding for CSVFormatter.save #17821

Licht-T · 2017-10-09T01:43:54Z

closes Bug: On Python 3 to_csv() encoding defaults to ascii if the dataframe contains special characters. #17097
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

codecov · 2017-10-09T02:41:22Z

Codecov Report

Merging #17821 into master will decrease coverage by 0.02%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master   #17821      +/-   ##
==========================================
- Coverage   91.26%   91.24%   -0.03%     
==========================================
  Files         163      163              
  Lines       49980    49985       +5     
==========================================
- Hits        45613    45607       -6     
- Misses       4367     4378      +11

Flag	Coverage Δ
#multiple	`89.04% <87.5%> (-0.01%)`	⬇️
#single	`40.24% <62.5%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/formats/format.py	`95.94% <87.5%> (-0.13%)`	⬇️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.74% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f0ee53...8263456. Read the comment docs.

codecov · 2017-10-09T02:41:29Z

Codecov Report

Merging #17821 into master will decrease coverage by 0.05%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master   #17821      +/-   ##
==========================================
- Coverage   91.26%    91.2%   -0.06%     
==========================================
  Files         163      163              
  Lines       49980    50019      +39     
==========================================
+ Hits        45613    45621       +8     
- Misses       4367     4398      +31

Flag	Coverage Δ
#multiple	`89.01% <87.5%> (-0.04%)`	⬇️
#single	`40.24% <62.5%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/formats/format.py	`95.94% <87.5%> (-0.13%)`	⬇️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/plotting/_compat.py	`62% <0%> (-6.19%)`	⬇️
pandas/plotting/_converter.py	`63.38% <0%> (-1.82%)`	⬇️
pandas/core/nanops.py	`96.67% <0%> (-0.99%)`	⬇️
pandas/core/frame.py	`97.77% <0%> (-0.08%)`	⬇️
pandas/core/panel.py	`96.94% <0%> (ø)`	⬆️
pandas/core/sparse/series.py	`95.26% <0%> (ø)`	⬆️
pandas/tests/indexes/conftest.py
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f0ee53...c222ee8. Read the comment docs.

gfyoung · 2017-10-09T03:12:23Z

pandas/io/formats/format.py

-                writer_kwargs['encoding'] = self.encoding
-                self.writer = UnicodeWriter(f, **writer_kwargs)
-            else:
+            if encoding is 'ascii':


String equality check instead of identity check (i.e. use ==).

gfyoung · 2017-10-09T03:13:07Z

pandas/tests/io/formats/test_to_csv.py

+        df = DataFrame({'col': [u"AAAAA", u"ÄÄÄÄÄ", u"ßßßßß", u"聞聞聞聞聞"]})
+
+        with tm.ensure_clean('test.csv') as path:
+            if pd.compat.PY2:


Add a comment about why we're raising an error and what should be done to avoid this error (i.e. the encoding argument parameter should be correct).

Licht-T · 2017-10-09T03:49:45Z

@gfyoung Thanks for your review. Fixed!

jschendel · 2017-10-09T05:31:03Z

pandas/io/formats/format.py

+            else:
+                encoding = 'utf-8'
+        else:
+            encoding = self.encoding


Not sure if this is overkill, but you can do encoding = self.encoding or ('ascii' if compat.PY2 else 'utf-8') to condense this whole block of if-else logic.

IMO, matter of taste here. I prefer the block-logic because its easier to read I find.

jreback · 2017-10-10T03:30:48Z

@Licht-T this looks good, can you add a whatsnew note (bug fixes section is ok).

also pls look and see if this change may close any/all of (if it does just indicate and include that issue in the note as well): #13068, #10813, #9712, #10755

jreback

see comments

TomAugspurger · 2017-10-10T15:44:06Z

AFAICT, none of those issues were closed by this one.

This reverts commit 039f2cf.

Licht-T · 2017-10-11T00:12:18Z

Sorry, I also added the whatsnew note

Licht-T · 2017-10-11T00:14:07Z

@jreback @TomAugspurger This PR does not solve these issue, I think.

TomAugspurger · 2017-10-11T15:19:51Z

The CI failure here was from the matplotlib 2.1 release. It'll be fixed when the merge commit is tested.

Thanks @Licht-T

* BUG: Fix default encoding for CSVFormatter.save * TST: Add to_csv defualt encoding test * DOC: Add comments on to_csv defualt encoding test * DOC: added release note * DOC: Add the fixing to_csv default encoding to whatsnew note * Revert "DOC: Add the fixing to_csv default encoding to whatsnew note" This reverts commit 039f2cf.

Licht-T force-pushed the fix-invalid-encoding-to_csv branch 3 times, most recently from 6e725bc to 8263456 Compare October 9, 2017 01:55

gfyoung reviewed Oct 9, 2017

View reviewed changes

gfyoung added Bug IO CSV read_csv, to_csv Unicode Unicode strings labels Oct 9, 2017

Licht-T added 3 commits October 9, 2017 12:41

BUG: Fix default encoding for CSVFormatter.save

cfb5e16

TST: Add to_csv defualt encoding test

a632835

DOC: Add comments on to_csv defualt encoding test

0cd7bf7

Licht-T force-pushed the fix-invalid-encoding-to_csv branch from 8263456 to 0cd7bf7 Compare October 9, 2017 03:49

jschendel reviewed Oct 9, 2017

View reviewed changes

jreback added this to the 0.21.0 milestone Oct 10, 2017

jreback requested changes Oct 10, 2017

View reviewed changes

DOC: added release note

08df290

TomAugspurger mentioned this pull request Oct 10, 2017

to_csv with lists of strings and unicode encoding produces wrong output #10813

Closed

Licht-T added 2 commits October 11, 2017 09:06

DOC: Add the fixing to_csv default encoding to whatsnew note

039f2cf

Revert "DOC: Add the fixing to_csv default encoding to whatsnew note"

c222ee8

This reverts commit 039f2cf.

TomAugspurger merged commit 9772c22 into pandas-dev:master Oct 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix default encoding for CSVFormatter.save #17821

BUG: Fix default encoding for CSVFormatter.save #17821

Licht-T commented Oct 9, 2017

codecov bot commented Oct 9, 2017

codecov bot commented Oct 9, 2017 •

edited

Loading

gfyoung Oct 9, 2017

gfyoung Oct 9, 2017

Licht-T commented Oct 9, 2017

jschendel Oct 9, 2017 •

edited

Loading

gfyoung Oct 9, 2017

jreback commented Oct 10, 2017

jreback left a comment

TomAugspurger commented Oct 10, 2017

Licht-T commented Oct 11, 2017

Licht-T commented Oct 11, 2017

TomAugspurger commented Oct 11, 2017

BUG: Fix default encoding for CSVFormatter.save #17821

BUG: Fix default encoding for CSVFormatter.save #17821

Conversation

Licht-T commented Oct 9, 2017

codecov bot commented Oct 9, 2017

Codecov Report

codecov bot commented Oct 9, 2017 • edited Loading

Codecov Report

gfyoung Oct 9, 2017

Choose a reason for hiding this comment

gfyoung Oct 9, 2017

Choose a reason for hiding this comment

Licht-T commented Oct 9, 2017

jschendel Oct 9, 2017 • edited Loading

Choose a reason for hiding this comment

gfyoung Oct 9, 2017

Choose a reason for hiding this comment

jreback commented Oct 10, 2017

jreback left a comment

Choose a reason for hiding this comment

TomAugspurger commented Oct 10, 2017

Licht-T commented Oct 11, 2017

Licht-T commented Oct 11, 2017

TomAugspurger commented Oct 11, 2017

codecov bot commented Oct 9, 2017 •

edited

Loading

jschendel Oct 9, 2017 •

edited

Loading