Skip to content

ENH: Allow compression in NDFrame.to_csv to be a dict with optional arguments (#26023) #26024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 41 commits into from
Aug 26, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
4e73dc4
ENH/BUG: Add arcname to to_csv for ZIP compressed csv filename (#26023)
drew-heenan Apr 8, 2019
ab7620d
DOC: Updated docs for arcname in NDFrame.to_csv (#26023)
drew-heenan Apr 8, 2019
2e782f9
conform to line length limit
drew-heenan Apr 8, 2019
83e8834
Fixed test_to_csv_zip_arcname for Windows paths
drew-heenan Apr 8, 2019
d238878
Merge remote-tracking branch 'upstream/master' into issue-26023
drew-heenan Apr 8, 2019
b41be54
to_csv compression may now be dict with possible keys 'method' and 'a…
drew-heenan Apr 9, 2019
60ea58c
test_to_csv_compression_dict uses compression_only fixture
drew-heenan Apr 9, 2019
8ba9082
delegate dict handling to _get_compression_method, type annotations
drew-heenan Apr 10, 2019
0a3a9fd
fix import order, None type annotations
drew-heenan Apr 10, 2019
a1cb3f7
compression args passed as kwargs, update relevant docs
drew-heenan Apr 14, 2019
af2a96c
style/doc improvements, change arcname to archive_name
drew-heenan Apr 15, 2019
5853a28
Merge branch 'master' into issue-26023
drew-heenan Apr 15, 2019
789751f
Merge branch 'master' into issue-26023
drew-heenan Apr 22, 2019
5b09e6f
add to_csv example, no method test, Optional types, tweaks; update wh…
drew-heenan Apr 22, 2019
68a2b4d
remove Index import type ignore
drew-heenan Apr 22, 2019
c856f50
Revert "remove Index import type ignore"
drew-heenan Apr 22, 2019
8df6c81
Merge remote-tracking branch 'upstream/master' into issue-26023
drew-heenan Apr 23, 2019
40d0252
Merge branch 'master' into issue-26023
drew-heenan Apr 26, 2019
18a735d
Improve docs/examples
drew-heenan May 5, 2019
103c877
Merge branch 'master' into issue-26023
drew-heenan May 5, 2019
b6c34bc
Merge remote-tracking branch 'upstream/master' into issue-26023
WillAyd Jun 8, 2019
969d387
Added back missed Callable import in generic
WillAyd Jun 8, 2019
abfbc0f
Merge remote-tracking branch 'upstream/master' into issue-26023
WillAyd Jun 9, 2019
04ae25d
Address comments
WillAyd Jun 9, 2019
9c22652
Typing cleanup
WillAyd Jun 9, 2019
56a75c2
Cleaned up docstring
WillAyd Jun 9, 2019
bbfea34
Merge remote-tracking branch 'upstream/master' into issue-26023
WillAyd Jun 23, 2019
7717f16
Merge remote-tracking branch 'upstream/master' into issue-26023
WillAyd Jul 15, 2019
779511e
blackify
WillAyd Jul 15, 2019
780eb04
Merge remote-tracking branch 'upstream/master' into issue-26023
WillAyd Jul 16, 2019
6c4e679
Added annotations where feasible
WillAyd Jul 16, 2019
1b567c9
Black and lint
WillAyd Jul 16, 2019
9324b63
Merge remote-tracking branch 'upstream/master' into issue-26023
WillAyd Jul 17, 2019
7cf65ee
isort fixup
WillAyd Jul 17, 2019
29374f3
Docstring fixup and more annotations
WillAyd Jul 17, 2019
6701aa4
Merge remote-tracking branch 'upstream/master' into issue-26023
WillAyd Aug 24, 2019
0f5489d
lint fixup
WillAyd Aug 24, 2019
e04138e
mypy fixup
WillAyd Aug 24, 2019
6f2bf00
whatsnew fixup
WillAyd Aug 25, 2019
865aa81
Annotation and doc fixups
WillAyd Aug 25, 2019
8d1deee
mypy typeshed bug fix
WillAyd Aug 25, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
style/doc improvements, change arcname to archive_name
  • Loading branch information
drew-heenan committed Apr 15, 2019
commit af2a96cd40bbc8ad797d13128fa5e7447189ff51
13 changes: 6 additions & 7 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -2920,10 +2920,10 @@ def to_latex(self, buf=None, columns=None, col_space=None, header=True,
def to_csv(self, path_or_buf=None, sep=",", na_rep='', float_format=None,
columns=None, header=True, index=True, index_label=None,
mode='w', encoding=None,
compression: Union[str, Dict, None] = 'infer', quoting=None,
quotechar='"', line_terminator=None, chunksize=None,
tupleize_cols=None, date_format=None, doublequote=True,
escapechar=None, decimal='.'):
compression: Union[str, Dict, None] = 'infer',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should always have types for Container classes like Dict. I think this should read Optional[Union[str, Dict[str, str]]]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right; I think it should be Optional[Union[str, Dict[str, Optional[str]]]], though, as both the method and archive name can be None.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the use case for an optional value in the dict? Isn't the point of accepting one in the first place for it to always have a method key with a str value associated with it?

Copy link
Contributor Author

@drew-heenan drew-heenan Apr 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WillAyd If there's some compression_method, including the Optional allows method to be None so we can have compression which is passed to to_csv be, for example,

compression = dict(method=compression_method, **other_args)

instead of something more cumbersome like

if compression_method is not None:
  compression = dict(method=compression_method, **other_args)
else:
  compression = None

which is admittedly not a huge difference, but passing None as either method or archive_name is supported regardless.

The dict keys can also be any kwargs of zipfile.ZipFile (see the BytesZipFile constructor), so some of those also expect strings, ints, bools and some are optional. I'd actually have the type be Optional[Union[str, Dict[str, Any]]]

quoting=None, quotechar='"', line_terminator=None,
chunksize=None, tupleize_cols=None, date_format=None,
doublequote=True, escapechar=None, decimal='.'):
r"""
Write object to a comma-separated values (csv) file.

Expand Down Expand Up @@ -2977,9 +2977,8 @@ def to_csv(self, path_or_buf=None, sep=",", na_rep='', float_format=None,
compression mode is 'infer' and `path_or_buf` is path-like, then
detect compression mode from the following extensions: '.gz',
'.bz2', '.zip' or '.xz'. (otherwise no compression). If dict given
and mode is 'zip' or inferred as 'zip', optional value at 'arcname'
specifies name of file within ZIP archive, assuming equal to
`path_or_buf` if not specified or None.
and mode is 'zip' or inferred as 'zip', other entries passed as
kwargs to ByteZipFile.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ByteZipFile is not user facing, see my comments above


.. versionchanged:: 0.25.0

Expand Down
22 changes: 12 additions & 10 deletions pandas/io/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import lzma
import mmap
import os
from typing import Dict, Tuple, Union
from typing import Any, Dict, Tuple, Union
from urllib.error import URLError # noqa
from urllib.parse import ( # noqa
urlencode, urljoin, urlparse as parse_url, uses_netloc, uses_params,
Expand Down Expand Up @@ -254,7 +254,7 @@ def _get_compression_method(compression: Union[str, Dict, None]):
------
ValueError on dict missing 'method' key
"""
compression_args = {} # type: Dict
compression_args = {} # type: Dict[str, Any]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this even necessary? Think creation in the conditional should suffice

Copy link
Contributor Author

@drew-heenan drew-heenan Apr 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WillAyd Is this in reference to the ValueError or compression_args?

# Handle dict
if isinstance(compression, dict):
compression_args = compression.copy()
Expand Down Expand Up @@ -319,8 +319,8 @@ def _infer_compression(filepath_or_buffer, compression):


def _get_handle(path_or_buf, mode, encoding=None,
compression: Union[str, Dict, None] = None, memory_map=False,
is_text=True):
compression: Union[str, Dict, None] = None,
memory_map=False, is_text=True):
"""
Get file handle for given path/buffer and mode.

Expand All @@ -338,8 +338,7 @@ def _get_handle(path_or_buf, mode, encoding=None,
and `filepath_or_buffer` is path-like, then detect compression from
the following extensions: '.gz', '.bz2', '.zip', or '.xz' (otherwise
no compression). If dict and compression mode is 'zip' or inferred as
'zip', optional value at key 'arcname' specifies the name of the file
within ZIP archive at `path_or_buf`.
'zip', other entries passed as kwargs to ByteZipFile.

.. versionchanged:: 0.25.0

Expand Down Expand Up @@ -466,15 +465,18 @@ class BytesZipFile(zipfile.ZipFile, BytesIO): # type: ignore
"""
# GH 17778
def __init__(self, file, mode, compression=zipfile.ZIP_DEFLATED,
arcname: Union[str, zipfile.ZipInfo, None] = None, **kwargs):
archive_name: Union[str, zipfile.ZipInfo, None] = None,
**kwargs):
if mode in ['wb', 'rb']:
mode = mode.replace('b', '')
self.arcname = arcname
self.archive_name = archive_name
super().__init__(file, mode, compression, **kwargs)

def write(self, data):
arcname = self.filename if self.arcname is None else self.arcname
super().writestr(arcname, data)
archive_name = self.filename
if self.archive_name is not None:
archive_name = self.archive_name
super().writestr(archive_name, data)

@property
def closed(self):
Expand Down
12 changes: 6 additions & 6 deletions pandas/io/formats/csvs.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,19 @@ class CSVFormatter(object):
def __init__(self, obj, path_or_buf=None, sep=",", na_rep='',
float_format=None, cols=None, header=True, index=True,
index_label=None, mode='w', nanRep=None, encoding=None,
compression: Union[str, Dict, None] = 'infer', quoting=None,
line_terminator='\n', chunksize=None, tupleize_cols=False,
quotechar='"', date_format=None, doublequote=True,
escapechar=None, decimal='.'):
compression: Union[str, Dict, None] = 'infer',
quoting=None, line_terminator='\n', chunksize=None,
tupleize_cols=False, quotechar='"', date_format=None,
doublequote=True, escapechar=None, decimal='.'):

self.obj = obj

if path_or_buf is None:
path_or_buf = StringIO()

# Extract compression mode as given, if dict
compression, self.compression_args \
= _get_compression_method(compression)
compression, self.compression_args = _get_compression_method(
compression)

self.path_or_buf, _, _, _ = get_filepath_or_buffer(
path_or_buf, encoding=encoding,
Expand Down
12 changes: 6 additions & 6 deletions pandas/tests/io/formats/test_to_csv.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,18 +550,18 @@ def test_to_csv_compression_dict(self, compression_only):
tm.assert_frame_equal(read_df, df)

@pytest.mark.parametrize("compression", ["zip", "infer"])
@pytest.mark.parametrize("arcname", [None, "test_to_csv.csv",
"test_to_csv.zip"])
def test_to_csv_zip_arcname(self, compression, arcname):
@pytest.mark.parametrize("archive_name", [None, "test_to_csv.csv",
"test_to_csv.zip"])
def test_to_csv_zip_arguments(self, compression, archive_name):
# GH 26023
from zipfile import ZipFile

df = DataFrame({"ABC": [1]})
with tm.ensure_clean("to_csv_arcname.zip") as path:
with tm.ensure_clean("to_csv_archive_name.zip") as path:
df.to_csv(path, compression={"method": compression,
"arcname": arcname})
"archive_name": archive_name})
zp = ZipFile(path)
expected_arcname = path if arcname is None else arcname
expected_arcname = path if archive_name is None else archive_name
expected_arcname = os.path.basename(expected_arcname)
assert len(zp.filelist) == 1
archived_file = os.path.basename(zp.filelist[0].filename)
Expand Down