BUG/REG: file-handle object handled incorrectly in to_csv #21478

minggli · 2018-06-14T17:03:53Z

closes #to_csv regression in 0.23.1 #21471
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

This error related to PR #21249 and #21227. This is never supported use case and to use file-handle in to_csv with compression, the file-object itself should be a compression archive such as:

with gzip.open('test.txt.gz', 'wt') as f:
    pd.DataFrame([0,1],index=['a','b'], columns=['c']).to_csv(f, sep='\t')

Regressed to 0.22 to_csv with support for zipfile. zipfile doesn't support writing csv strings to a zip archive using a file-handle. So buffer is used to catch the writing and dump into zip archive in one go. The other scenarios remain unchanged.

codecov · 2018-06-14T17:54:53Z

Codecov Report

Merging #21478 into master will increase coverage by 0.03%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master   #21478      +/-   ##
==========================================
+ Coverage   91.89%   91.92%   +0.03%     
==========================================
  Files         153      153              
  Lines       49604    49599       -5     
==========================================
+ Hits        45584    45595      +11     
+ Misses       4020     4004      -16

Flag	Coverage Δ
#multiple	`90.32% <95.65%> (+0.03%)`	⬆️
#single	`41.88% <43.47%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/frame.py	`97.23% <ø> (ø)`	⬆️
pandas/core/series.py	`94.17% <ø> (ø)`	⬆️
pandas/io/common.py	`70.28% <100%> (+0.24%)`	⬆️
pandas/io/formats/csvs.py	`97.67% <95.23%> (-0.48%)`	⬇️
pandas/core/indexes/base.py	`96.62% <0%> (ø)`	⬆️
pandas/core/algorithms.py	`94.83% <0%> (+0.01%)`	⬆️
pandas/io/json/json.py	`92.47% <0%> (+0.23%)`	⬆️
pandas/util/testing.py	`85.96% <0%> (+1.35%)`	⬆️

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 bf1c3dc...cf3afac. Read the comment docs.

littleK0i · 2018-06-14T21:03:06Z

I am not a proper reviewer, but I am suspicious about the whole "ZipFile magic" part.

It looks really ugly to me, and this code may potentially cause spike of 2x memory usage due to StringIO accumulation and "getvalue()" call.

I do not fully understand what was the problem with 0.22 implementation. Yes, "read_csv" and "to_csv" were inconsistent, but it contained less "magic", and users could always wrap anything they need with few lines of code.

Also, compression algos usually have additional parameters. Users might want to change defaults later on (e.g. compression level), and they'll need manual wrapping for that anyway.

Just thoughts.

minggli · 2018-06-14T23:28:30Z

agree that for extremely large zip output, memory usage will increase when writing csv to StringIO first and then call getvalue(). With 8G RAM MacOS, enough to write a [150m, 3] dataframe of 3.4g memory. the memory increase from getvalue call is only when zip compression, for other use cases, there should be no difference as 0.22. When path_or_buf is None, to_csv uses StringIO and call getvalue() already.

pandas/pandas/core/frame.py

Line 1747 in bf1c3dc

if path_or_buf is None:

the custom class just provides a file-like that accepts string into zip archive when ZipFile class doesn't really offer that.

The ability to write zip archive is added #17778 where only read zip was supported, whereas we have round-trip ability for others. the motivation is consistency but also zip compression format is quite common and strange not to be supported.

the zip class is not only for to_csv but provides utility for to_json and to_pickle case too.

jreback

looks good. mostly doc comments. ping on green.

jreback · 2018-06-15T17:08:18Z

doc/source/whatsnew/v0.23.2.txt

@@ -56,7 +56,7 @@ Bug Fixes

 **I/O**

-
+- Bug in :meth:`to_csv` when handling file-like object incorrectly (:issue:`21471`)


can you move to the regression section

jreback · 2018-06-15T17:08:24Z

pandas/io/formats/csvs.py

-            # path_or_buf is file handle
-            path_or_buf = self.path_or_buf.name
+        if self.compression and hasattr(self.path_or_buf, 'write'):
+            import warnings


you can move this import to the top

jreback · 2018-06-15T17:09:01Z

pandas/io/formats/csvs.py

-        elif hasattr(self.path_or_buf, 'name'):
-            # path_or_buf is file handle
-            path_or_buf = self.path_or_buf.name
+        if self.compression and hasattr(self.path_or_buf, 'write'):


can uou add a comment here (sure the warning says it all, but helpful nonetheless), also an issue reference

jreback · 2018-06-15T17:09:04Z

pandas/io/formats/csvs.py

+                   "object as input.")
+            warnings.warn(msg, RuntimeWarning, stacklevel=2)
+
+        if isinstance(self.path_or_buf, ZipFile) or (


you can just do

is_zip = isinstance(......) or (.....)

jreback · 2018-06-15T17:09:23Z

pandas/io/formats/csvs.py

+                   "object as input.")
+            warnings.warn(msg, RuntimeWarning, stacklevel=2)
+
+        if isinstance(self.path_or_buf, ZipFile) or (


comment here on what is_zip means

jreback · 2018-06-15T17:09:29Z

pandas/io/formats/csvs.py

+                try:
+                    self.path_or_buf.write(buf)
+                except AttributeError:
+                    f, handles = _get_handle(self.path_or_buf, self.mode,


can you add a comment on when the except happens?

jreback · 2018-06-15T17:09:34Z

pandas/tests/series/test_io.py

+            f, _handles = _get_handle(filename, 'w', compression=compression,
+                                      encoding=encoding)
+            with f:
+                s.to_csv(f, encoding=encoding, header=True)
            result_fh = pd.read_csv(filename, compression=compression,


you can rename this to result (and below)

jreback · 2018-06-15T17:09:54Z

pandas/tests/test_common.py

+
+
+def test_compression_warning(compression_only):
+    df = DataFrame(100 * [[0.123456, 0.234567, 0.567567],


can you add the issue number

pep8speaks · 2018-06-15T19:31:52Z

Hello @minggli! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on June 16, 2018 at 13:40 Hours UTC

gfyoung · 2018-06-16T07:31:17Z

pandas/io/formats/csvs.py

+                # GH 17778 handles zip compression separately.
+                buf = f.getvalue()
+                try:
+                    self.path_or_buf.write(buf)


They say that it's sometimes easier to ask for forgiveness than it is for permission, hence the try/except block that you choose here. That being said, your treatment of self.path_or_buf.write is inconsistent here compared to:

https://github.com/pandas-dev/pandas/pull/21478/files#diff-eaa887c826bfb361d98db1ddb668c7deR150

Where you ask for permission instead of forgiveness. In addition, the logic seems somewhat similar. Can we potentially deduplicate it?

see your point. changed to if else to make it consistent.

gfyoung · 2018-06-16T07:32:15Z

doc/source/whatsnew/v0.23.2.txt

@@ -16,7 +16,7 @@ and bug fixes. We recommend that all users upgrade to this version.
 Fixed Regressions
 ~~~~~~~~~~~~~~~~~

-
+- Fixed Regression in :meth:`to_csv` when handling file-like object incorrectly (:issue:`21471`)


nit: "Regression" --> "regression"

minggli · 2018-06-16T12:04:37Z

@jreback comments carried out.

jreback · 2018-06-18T22:45:38Z

thanks @minggli nice followup patch!

jreback · 2018-06-18T22:48:00Z

@TomAugspurger this might be tricky to backport

minggli · 2018-06-19T09:29:48Z

@jreback @gfyoung happy to help. thanks for reviewing.

(cherry picked from commit 91451cb)

…#21478)

minggli added 7 commits June 14, 2018 15:03

implement closed attribute for PY2

805739e

revert PR 21249

1185281

revert PR 21249

5e66ee2

use _get_handle to produce a handle

d11dcae

use _get_handle to produce file handle

d3dac82

regression to 0.22 and add zipfile support

9567900

add warning when passing file handle with compression flag

6accf04

jorisvandenbossche mentioned this pull request Jun 14, 2018

to_csv regression in 0.23.1 #21471

Closed

jorisvandenbossche added this to the 0.23.2 milestone Jun 14, 2018

jorisvandenbossche added Regression Functionality that used to work in a prior pandas version IO CSV read_csv, to_csv labels Jun 14, 2018

catch zip

d5d976e

minggli force-pushed the bugfix/to_csv branch from 0da6c41 to d5d976e Compare June 14, 2018 18:17

minggli added 2 commits June 14, 2018 19:35

add warnings when passing file-object and compression parameter

05bbd81

catch file-handle and zip at the same time.

6f5bcf1

add whatsnew

c754329

jreback requested changes Jun 15, 2018

View reviewed changes

jreback requested a review from WillAyd June 15, 2018 17:10

minggli added 4 commits June 15, 2018 20:05

move section in whatsnew

23317af

move import to the top

ab37993

rename variables

5392bcc

comments

f34ab4e

pep8

08f8972

minggli closed this Jun 15, 2018

minggli reopened this Jun 15, 2018

gfyoung reviewed Jun 16, 2018

View reviewed changes

minggli added 2 commits June 16, 2018 10:24

minor styling

689f011

repeat same pattern for simplicity

c472a5b

consistent closed attribtue

cf3afac

gfyoung approved these changes Jun 16, 2018

View reviewed changes

jreback added the Needs Backport label Jun 18, 2018

jreback approved these changes Jun 18, 2018

View reviewed changes

jreback merged commit 91451cb into pandas-dev:master Jun 18, 2018

minggli deleted the bugfix/to_csv branch June 19, 2018 07:01

WillAyd mentioned this pull request Jun 20, 2018

Test Case for to_csv to sys.stdout #21561

Closed

Liam3851 mentioned this pull request Jun 20, 2018

Pandas version 0.23.1 failed to save dataframe onto HDFS with hdfs cli #21560

Closed

jorisvandenbossche removed the Needs Backport label Jun 29, 2018

jorisvandenbossche pushed a commit that referenced this pull request Jun 29, 2018

BUG/REG: file-handle object handled incorrectly in to_csv (#21478)

cfc787e

(cherry picked from commit 91451cb)

jorisvandenbossche pushed a commit that referenced this pull request Jul 2, 2018

BUG/REG: file-handle object handled incorrectly in to_csv (#21478)

2292005

(cherry picked from commit 91451cb)

JacobHayes mentioned this pull request Jul 7, 2018

to_csv 'w' mode doesn't truncate for StringIO #21748

Closed

dhimmel mentioned this pull request Jul 21, 2018

Default to_* methods to compression='infer' #22011

Merged

4 tasks

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

BUG/REG: file-handle object handled incorrectly in to_csv (pandas-dev…

dae6e4c

…#21478)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG/REG: file-handle object handled incorrectly in to_csv #21478

BUG/REG: file-handle object handled incorrectly in to_csv #21478

minggli commented Jun 14, 2018 •

edited

Loading

codecov bot commented Jun 14, 2018 •

edited

Loading

littleK0i commented Jun 14, 2018 •

edited

Loading

minggli commented Jun 14, 2018 •

edited

Loading

jreback left a comment

jreback Jun 15, 2018

minggli Jun 15, 2018

jreback Jun 15, 2018

jreback Jun 15, 2018

jreback Jun 15, 2018

jreback Jun 15, 2018

jreback Jun 15, 2018

minggli Jun 15, 2018

jreback Jun 15, 2018

minggli Jun 15, 2018

jreback Jun 15, 2018

minggli Jun 15, 2018

pep8speaks commented Jun 15, 2018 •

edited

Loading

gfyoung Jun 16, 2018 •

edited

Loading

minggli Jun 16, 2018

gfyoung Jun 16, 2018

minggli Jun 16, 2018

minggli commented Jun 16, 2018

jreback commented Jun 18, 2018

jreback commented Jun 18, 2018

minggli commented Jun 19, 2018



		def test_compression_warning(compression_only):
		df = DataFrame(100 * [[0.123456, 0.234567, 0.567567],

BUG/REG: file-handle object handled incorrectly in to_csv #21478

BUG/REG: file-handle object handled incorrectly in to_csv #21478

Conversation

minggli commented Jun 14, 2018 • edited Loading

codecov bot commented Jun 14, 2018 • edited Loading

Codecov Report

littleK0i commented Jun 14, 2018 • edited Loading

minggli commented Jun 14, 2018 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Jun 15, 2018 • edited Loading

Comment last updated on June 16, 2018 at 13:40 Hours UTC

gfyoung Jun 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minggli commented Jun 16, 2018

jreback commented Jun 18, 2018

jreback commented Jun 18, 2018

minggli commented Jun 19, 2018

minggli commented Jun 14, 2018 •

edited

Loading

codecov bot commented Jun 14, 2018 •

edited

Loading

littleK0i commented Jun 14, 2018 •

edited

Loading

minggli commented Jun 14, 2018 •

edited

Loading

pep8speaks commented Jun 15, 2018 •

edited

Loading

gfyoung Jun 16, 2018 •

edited

Loading