From 7bbe9104ed2fabe25a5d61ede8e5afb9153616c7 Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Mon, 11 Feb 2019 21:54:39 +0100 Subject: [PATCH 01/12] Address comments of @gfyoung in #25153 --- pandas/io/excel/_base.py | 5 ++--- pandas/io/excel/_util.py | 12 +++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pandas/io/excel/_base.py b/pandas/io/excel/_base.py index ed5943e9a1698..8f7bf8e0466f9 100644 --- a/pandas/io/excel/_base.py +++ b/pandas/io/excel/_base.py @@ -590,9 +590,8 @@ def __new__(cls, path, engine=None, **kwargs): if engine == 'auto': engine = _get_default_writer(ext) except KeyError: - error = ValueError("No engine for filetype: '{ext}'" - .format(ext=ext)) - raise error + raise ValueError("No engine for filetype: '{ext}'" + .format(ext=ext)) cls = get_writer(engine) return object.__new__(cls) diff --git a/pandas/io/excel/_util.py b/pandas/io/excel/_util.py index 1aeaf70f0832e..d8f1f6e370b48 100644 --- a/pandas/io/excel/_util.py +++ b/pandas/io/excel/_util.py @@ -7,7 +7,9 @@ from pandas.core import config -_writer_extensions = ["xlsx", "xls", "xlsm"] + +# the following extensions are already registered in pandas/core/config_init.py +_registered_writer_extensions = ["xlsx", "xls", "xlsm"] _writers = {} @@ -24,13 +26,15 @@ def register_writer(klass): for ext in klass.supported_extensions: if ext.startswith('.'): ext = ext[1:] - if ext not in _writer_extensions: + if ext not in _registered_writer_extensions: config.register_option("io.excel.{ext}.writer".format(ext=ext), engine_name, validator=str) - _writer_extensions.append(ext) + _registered_writer_extensions.append(ext) def _get_default_writer(ext): + """Return the default writer per extension. This default engine is used + unles another engine is explicitly defined.""" _default_writers = {'xlsx': 'openpyxl', 'xlsm': 'openpyxl', 'xls': 'xlwt'} try: import xlsxwriter # noqa @@ -230,8 +234,6 @@ def _fill_mi_header(row, control_row): return _maybe_convert_to_string(row), control_row -# fill blank if index_col not None - def _pop_header_name(row, index_col): """ From c98f72ed0efab412e3d221dc8a9258ffaba7cc27 Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Mon, 11 Feb 2019 22:45:32 +0100 Subject: [PATCH 02/12] satisfy isort --- pandas/io/excel/_util.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/io/excel/_util.py b/pandas/io/excel/_util.py index d8f1f6e370b48..140c6f63366de 100644 --- a/pandas/io/excel/_util.py +++ b/pandas/io/excel/_util.py @@ -7,7 +7,6 @@ from pandas.core import config - # the following extensions are already registered in pandas/core/config_init.py _registered_writer_extensions = ["xlsx", "xls", "xlsm"] From bc074edea0ef91f7fb69c1191b133dc65366bbcf Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Tue, 12 Feb 2019 08:17:16 +0100 Subject: [PATCH 03/12] removed seemingly dead code as per suggestion @WillAyd --- pandas/io/excel/_util.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/pandas/io/excel/_util.py b/pandas/io/excel/_util.py index 140c6f63366de..ce4b19a192652 100644 --- a/pandas/io/excel/_util.py +++ b/pandas/io/excel/_util.py @@ -5,12 +5,6 @@ from pandas.core.dtypes.common import is_integer, is_list_like -from pandas.core import config - -# the following extensions are already registered in pandas/core/config_init.py -_registered_writer_extensions = ["xlsx", "xls", "xlsm"] - - _writers = {} @@ -22,13 +16,6 @@ def register_writer(klass): raise ValueError("Can only register callables as engines") engine_name = klass.engine _writers[engine_name] = klass - for ext in klass.supported_extensions: - if ext.startswith('.'): - ext = ext[1:] - if ext not in _registered_writer_extensions: - config.register_option("io.excel.{ext}.writer".format(ext=ext), - engine_name, validator=str) - _registered_writer_extensions.append(ext) def _get_default_writer(ext): From 62ce7db520ee6c1b40e5e58b56d8c23da70ec866 Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Tue, 12 Feb 2019 16:56:37 +0100 Subject: [PATCH 04/12] remove test for registering custom excel extensions --- pandas/tests/io/test_excel.py | 40 +---------------------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/pandas/tests/io/test_excel.py b/pandas/tests/io/test_excel.py index 8c92db734168b..2415497ff8fa4 100644 --- a/pandas/tests/io/test_excel.py +++ b/pandas/tests/io/test_excel.py @@ -22,7 +22,7 @@ from pandas.io.common import URLError from pandas.io.excel import ( ExcelFile, ExcelWriter, _OpenpyxlWriter, _XlsxWriter, _XlwtWriter, - read_excel, register_writer) + read_excel) from pandas.io.formats.excel import ExcelFormatter from pandas.io.parsers import read_csv @@ -2350,44 +2350,6 @@ def test_ExcelWriter_dispatch_raises(self): with pytest.raises(ValueError, match='No engine'): ExcelWriter('nothing') - @pytest.mark.filterwarnings("ignore:\\nPanel:FutureWarning") - def test_register_writer(self): - # some awkward mocking to test out dispatch and such actually works - called_save = [] - called_write_cells = [] - - class DummyClass(ExcelWriter): - called_save = False - called_write_cells = False - supported_extensions = ['test', 'xlsx', 'xls'] - engine = 'dummy' - - def save(self): - called_save.append(True) - - def write_cells(self, *args, **kwargs): - called_write_cells.append(True) - - def check_called(func): - func() - assert len(called_save) >= 1 - assert len(called_write_cells) >= 1 - del called_save[:] - del called_write_cells[:] - - with pd.option_context('io.excel.xlsx.writer', 'dummy'): - register_writer(DummyClass) - writer = ExcelWriter('something.test') - assert isinstance(writer, DummyClass) - df = tm.makeCustomDataframe(1, 1) - - func = lambda: df.to_excel('something.test') - check_called(func) - check_called(lambda: df.to_excel('something.xlsx')) - check_called( - lambda: df.to_excel( - 'something.xls', engine='dummy')) - @pytest.mark.parametrize('engine', [ pytest.param('xlwt', From 5e49439bd15a1c80e02a42154fa8501ae7773147 Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Wed, 13 Feb 2019 08:34:19 +0100 Subject: [PATCH 05/12] Revert "remove test for registering custom excel extensions" This reverts commit 62ce7db520ee6c1b40e5e58b56d8c23da70ec866. --- pandas/tests/io/test_excel.py | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/pandas/tests/io/test_excel.py b/pandas/tests/io/test_excel.py index 2415497ff8fa4..8c92db734168b 100644 --- a/pandas/tests/io/test_excel.py +++ b/pandas/tests/io/test_excel.py @@ -22,7 +22,7 @@ from pandas.io.common import URLError from pandas.io.excel import ( ExcelFile, ExcelWriter, _OpenpyxlWriter, _XlsxWriter, _XlwtWriter, - read_excel) + read_excel, register_writer) from pandas.io.formats.excel import ExcelFormatter from pandas.io.parsers import read_csv @@ -2350,6 +2350,44 @@ def test_ExcelWriter_dispatch_raises(self): with pytest.raises(ValueError, match='No engine'): ExcelWriter('nothing') + @pytest.mark.filterwarnings("ignore:\\nPanel:FutureWarning") + def test_register_writer(self): + # some awkward mocking to test out dispatch and such actually works + called_save = [] + called_write_cells = [] + + class DummyClass(ExcelWriter): + called_save = False + called_write_cells = False + supported_extensions = ['test', 'xlsx', 'xls'] + engine = 'dummy' + + def save(self): + called_save.append(True) + + def write_cells(self, *args, **kwargs): + called_write_cells.append(True) + + def check_called(func): + func() + assert len(called_save) >= 1 + assert len(called_write_cells) >= 1 + del called_save[:] + del called_write_cells[:] + + with pd.option_context('io.excel.xlsx.writer', 'dummy'): + register_writer(DummyClass) + writer = ExcelWriter('something.test') + assert isinstance(writer, DummyClass) + df = tm.makeCustomDataframe(1, 1) + + func = lambda: df.to_excel('something.test') + check_called(func) + check_called(lambda: df.to_excel('something.xlsx')) + check_called( + lambda: df.to_excel( + 'something.xls', engine='dummy')) + @pytest.mark.parametrize('engine', [ pytest.param('xlwt', From cf3c5bc1b08941e4cd681d27beb2a26c25d400f8 Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Wed, 13 Feb 2019 08:48:04 +0100 Subject: [PATCH 06/12] Remove test for registering excel file with custom extension as this functionality is removed --- pandas/tests/io/test_excel.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pandas/tests/io/test_excel.py b/pandas/tests/io/test_excel.py index 8c92db734168b..553b1ad225a0e 100644 --- a/pandas/tests/io/test_excel.py +++ b/pandas/tests/io/test_excel.py @@ -2359,7 +2359,7 @@ def test_register_writer(self): class DummyClass(ExcelWriter): called_save = False called_write_cells = False - supported_extensions = ['test', 'xlsx', 'xls'] + supported_extensions = ['xlsx', 'xls'] engine = 'dummy' def save(self): @@ -2377,12 +2377,10 @@ def check_called(func): with pd.option_context('io.excel.xlsx.writer', 'dummy'): register_writer(DummyClass) - writer = ExcelWriter('something.test') + writer = ExcelWriter('something.xlsx') assert isinstance(writer, DummyClass) df = tm.makeCustomDataframe(1, 1) - func = lambda: df.to_excel('something.test') - check_called(func) check_called(lambda: df.to_excel('something.xlsx')) check_called( lambda: df.to_excel( From e42b10c3b3559cdb190b6c3f26569dd8bf241731 Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Wed, 13 Feb 2019 09:47:40 +0100 Subject: [PATCH 07/12] trigger test rerun --- pandas/tests/io/test_excel.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/tests/io/test_excel.py b/pandas/tests/io/test_excel.py index 553b1ad225a0e..8a71e5cecf819 100644 --- a/pandas/tests/io/test_excel.py +++ b/pandas/tests/io/test_excel.py @@ -2380,7 +2380,6 @@ def check_called(func): writer = ExcelWriter('something.xlsx') assert isinstance(writer, DummyClass) df = tm.makeCustomDataframe(1, 1) - check_called(lambda: df.to_excel('something.xlsx')) check_called( lambda: df.to_excel( From a82119ad94395f8dce68d8322619118df97fbb46 Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Thu, 14 Feb 2019 09:46:13 +0100 Subject: [PATCH 08/12] improve docstring --- pandas/io/excel/_util.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/pandas/io/excel/_util.py b/pandas/io/excel/_util.py index ce4b19a192652..80650e366796a 100644 --- a/pandas/io/excel/_util.py +++ b/pandas/io/excel/_util.py @@ -19,8 +19,20 @@ def register_writer(klass): def _get_default_writer(ext): - """Return the default writer per extension. This default engine is used - unles another engine is explicitly defined.""" + """ + Return the default writer for the given extension. + + Parameters + ---------- + ext : str + The excel file extension for which to get the default engine. + + Returns + ------- + engine : str + The default engine for the extension. + + """ _default_writers = {'xlsx': 'openpyxl', 'xlsm': 'openpyxl', 'xls': 'xlwt'} try: import xlsxwriter # noqa From 7f816754ea72d478445af0c1c54e31cf4466c289 Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Thu, 14 Feb 2019 09:52:17 +0100 Subject: [PATCH 09/12] imprve docstring --- pandas/io/excel/_util.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/pandas/io/excel/_util.py b/pandas/io/excel/_util.py index 80650e366796a..3e0b339ffe731 100644 --- a/pandas/io/excel/_util.py +++ b/pandas/io/excel/_util.py @@ -9,9 +9,21 @@ def register_writer(klass): - """Adds engine to the excel writer registry. You must use this method to - integrate with ``to_excel``. Also adds config options for any new - ``supported_extensions`` defined on the writer.""" + """ + Add engine to the excel writer registry. You must use this method to + integrate with ``to_excel``. + + Parameters + ---------- + klass : class + Instance of ExcelWriter + + Returns + ------- + None + + """ + if not callable(klass): raise ValueError("Can only register callables as engines") engine_name = klass.engine From f30ab5c6663fb8b72182faf654135fcd1709093f Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Sun, 17 Feb 2019 07:36:35 +0100 Subject: [PATCH 10/12] remove trailing whitespace --- pandas/io/excel/_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/io/excel/_util.py b/pandas/io/excel/_util.py index 3e0b339ffe731..2f8483a865c76 100644 --- a/pandas/io/excel/_util.py +++ b/pandas/io/excel/_util.py @@ -11,7 +11,7 @@ def register_writer(klass): """ Add engine to the excel writer registry. You must use this method to - integrate with ``to_excel``. + integrate with ``to_excel``. Parameters ---------- From 19160fae6109076cb38fe27125adcdc42ef1f20b Mon Sep 17 00:00:00 2001 From: William Ayd Date: Tue, 19 Feb 2019 09:16:11 +0100 Subject: [PATCH 11/12] Apply suggestions from code review Co-Authored-By: tdamsma --- pandas/io/excel/_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/io/excel/_util.py b/pandas/io/excel/_util.py index 2f8483a865c76..1f0175ae9b5a0 100644 --- a/pandas/io/excel/_util.py +++ b/pandas/io/excel/_util.py @@ -41,7 +41,7 @@ def _get_default_writer(ext): Returns ------- - engine : str + str The default engine for the extension. """ From bcf053e260b028d989efd482d200d1d737cf9b7a Mon Sep 17 00:00:00 2001 From: Thijs Damsma Date: Tue, 19 Feb 2019 09:33:13 +0100 Subject: [PATCH 12/12] Improve docstrings as per WillAyd's suggestions --- pandas/io/excel/_util.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/pandas/io/excel/_util.py b/pandas/io/excel/_util.py index 1f0175ae9b5a0..49255d83d1cd3 100644 --- a/pandas/io/excel/_util.py +++ b/pandas/io/excel/_util.py @@ -10,20 +10,14 @@ def register_writer(klass): """ - Add engine to the excel writer registry. You must use this method to - integrate with ``to_excel``. + Add engine to the excel writer registry.io.excel. + + You must use this method to integrate with ``to_excel``. Parameters ---------- - klass : class - Instance of ExcelWriter - - Returns - ------- - None - + klass : ExcelWriter """ - if not callable(klass): raise ValueError("Can only register callables as engines") engine_name = klass.engine @@ -43,7 +37,6 @@ def _get_default_writer(ext): ------- str The default engine for the extension. - """ _default_writers = {'xlsx': 'openpyxl', 'xlsm': 'openpyxl', 'xls': 'xlwt'} try: