From 2547a0281063e60f1378f08ce6cfcc601752e82e Mon Sep 17 00:00:00 2001 From: Richard Shadrach Date: Tue, 1 Feb 2022 22:54:52 -0500 Subject: [PATCH 1/5] DEP: Deprecate ExcelWriter attributes --- doc/source/reference/io.rst | 1 - doc/source/whatsnew/v1.5.0.rst | 13 ++ pandas/io/excel/_base.py | 165 ++++++++++++++++++++----- pandas/io/excel/_odswriter.py | 16 ++- pandas/io/excel/_openpyxl.py | 38 +++--- pandas/io/excel/_xlsxwriter.py | 14 ++- pandas/io/excel/_xlwt.py | 20 ++- pandas/io/formats/excel.py | 2 +- pandas/tests/io/excel/test_openpyxl.py | 4 +- pandas/tests/io/excel/test_writers.py | 7 +- 10 files changed, 217 insertions(+), 63 deletions(-) diff --git a/doc/source/reference/io.rst b/doc/source/reference/io.rst index 44ee09f2a5e6b..70fd381bffd2c 100644 --- a/doc/source/reference/io.rst +++ b/doc/source/reference/io.rst @@ -53,7 +53,6 @@ Excel .. autosummary:: :toctree: api/ - :template: autosummary/class_without_autosummary.rst ExcelWriter diff --git a/doc/source/whatsnew/v1.5.0.rst b/doc/source/whatsnew/v1.5.0.rst index 3829306f77a0b..a70c1c7dda539 100644 --- a/doc/source/whatsnew/v1.5.0.rst +++ b/doc/source/whatsnew/v1.5.0.rst @@ -183,6 +183,19 @@ use ``series.loc[i:j]``. Slicing on a :class:`DataFrame` will not be affected. +.. _whatsnew_150.deprecations.excel_writer_attributes: + +:class:`ExcelWriter` attributes +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The attributes and methods of :class:`ExcelWriter` were previously documented as not +being public. However some third party Excel engines documented accessing +``ExcelWriter.book`` or ``ExcelWriter.sheets``, and users were utilizing these +and possibly other attributes. In order to support this, pandas has made some +attributes and methods public. Other attributes and methods have been deprecated +and will raise a ``FutureWarning`` when accessed as they will be removed in a future +version. See the documentation of :class:`ExcelWriter` for further details. + .. _whatsnew_150.deprecations.other: Other Deprecations diff --git a/pandas/io/excel/_base.py b/pandas/io/excel/_base.py index b33955737a111..a88fab48eed5e 100644 --- a/pandas/io/excel/_base.py +++ b/pandas/io/excel/_base.py @@ -836,18 +836,8 @@ class ExcelWriter(metaclass=abc.ABCMeta): Use engine_kwargs instead. - Attributes - ---------- - None - - Methods - ------- - None - Notes ----- - None of the methods and properties are considered public. - For compatibility with CSV writers, ExcelWriter serializes lists and dicts to strings before writing. @@ -1034,7 +1024,7 @@ def __new__( return object.__new__(cls) # declare external properties you can count on - path = None + _path = None @property @abc.abstractmethod @@ -1054,7 +1044,15 @@ def sheets(self) -> dict[str, Any]: """Mapping of sheet names to sheet objects.""" pass + @property @abc.abstractmethod + def book(self): + """Book instance. Class type will depend on the engine used. + + This attribute can be used to access engine-specific features. + """ + pass + def write_cells( self, cells, @@ -1066,6 +1064,8 @@ def write_cells( """ Write given formatted cells into Excel an excel sheet + .. deprecated:: 1.5.0 + Parameters ---------- cells : generator @@ -1077,12 +1077,47 @@ def write_cells( freeze_panes: int tuple of length 2 contains the bottom-most row and right-most column to freeze """ - pass + self._deprecate("write_cells") + return self._write_cells(cells, sheet_name, startrow, startcol, freeze_panes) @abc.abstractmethod + def _write_cells( + self, + cells, + sheet_name: str | None = None, + startrow: int = 0, + startcol: int = 0, + freeze_panes: tuple[int, int] | None = None, + ) -> None: + """ + Write given formatted cells into Excel an excel sheet + + Parameters + ---------- + cells : generator + cell of formatted data to save to Excel sheet + sheet_name : str, default None + Name of Excel sheet, if None, then use self.cur_sheet + startrow : upper left cell row to dump data frame + startcol : upper left cell column to dump data frame + freeze_panes: int tuple of length 2 + contains the bottom-most row and right-most column to freeze + """ + pass + def save(self) -> None: """ Save workbook to disk. + + .. deprecated:: 1.5.0 + """ + self._deprecate("save") + return self._save() + + @abc.abstractmethod + def _save(self) -> None: + """ + Save workbook to disk. """ pass @@ -1101,7 +1136,7 @@ def __init__( # validate that this engine can handle the extension if isinstance(path, str): ext = os.path.splitext(path)[-1] - self.check_extension(ext) + self._check_extension(ext) # use mode to open the file if "b" not in mode: @@ -1111,25 +1146,25 @@ def __init__( mode = mode.replace("a", "r+") # cast ExcelWriter to avoid adding 'if self.handles is not None' - self.handles = IOHandles( + self._handles = IOHandles( cast(IO[bytes], path), compression={"compression": None} ) if not isinstance(path, ExcelWriter): - self.handles = get_handle( + self._handles = get_handle( path, mode, storage_options=storage_options, is_text=False ) - self.cur_sheet = None + self._cur_sheet = None if date_format is None: - self.date_format = "YYYY-MM-DD" + self._date_format = "YYYY-MM-DD" else: - self.date_format = date_format + self._date_format = date_format if datetime_format is None: - self.datetime_format = "YYYY-MM-DD HH:MM:SS" + self._datetime_format = "YYYY-MM-DD HH:MM:SS" else: - self.datetime_format = datetime_format + self._datetime_format = datetime_format - self.mode = mode + self._mode = mode if if_sheet_exists not in (None, "error", "new", "replace", "overlay"): raise ValueError( @@ -1140,16 +1175,78 @@ def __init__( raise ValueError("if_sheet_exists is only valid in append mode (mode='a')") if if_sheet_exists is None: if_sheet_exists = "error" - self.if_sheet_exists = if_sheet_exists + self._if_sheet_exists = if_sheet_exists + + def _deprecate(self, attr: str): + """ + Deprecate attribute or method for ExcelWriter. + """ + warnings.warn( + f"{attr} is not part of the public API, usage can give in unexpected " + "results and will be removed in a future version", + FutureWarning, + stacklevel=find_stack_level(), + ) + + @property + def date_format(self) -> str: + """ + Format string for dates written into Excel files (e.g. ‘YYYY-MM-DD’). + """ + return self._date_format + + @property + def datetime_format(self) -> str: + """ + Format string for dates written into Excel files (e.g. ‘YYYY-MM-DD’). + """ + return self._datetime_format + + @property + def if_sheet_exists(self) -> str: + """ + How to behave when writing to a sheet that already exists in append mode. + """ + return self._if_sheet_exists + + @property + def cur_sheet(self): + """ + Current sheet for writing. + + .. deprecated:: 1.5.0 + """ + self._deprecate("cur_sheet") + return self._cur_sheet + + @property + def handles(self): + """ + Handles to Excel sheets. + + .. deprecated:: 1.5.0 + """ + self._deprecate("handles") + return self._handles + + @property + def path(self): + """ + Path to Excel file. + + .. deprecated:: 1.5.0 + """ + self._deprecate("path") + return self._path def __fspath__(self): - return getattr(self.handles.handle, "name", "") + return getattr(self._handles.handle, "name", "") def _get_sheet_name(self, sheet_name: str | None) -> str: if sheet_name is None: - sheet_name = self.cur_sheet + sheet_name = self._cur_sheet if sheet_name is None: # pragma: no cover - raise ValueError("Must pass explicit sheet_name or set cur_sheet property") + raise ValueError("Must pass explicit sheet_name or set _cur_sheet property") return sheet_name def _value_with_fmt(self, val) -> tuple[object, str | None]: @@ -1175,9 +1272,9 @@ def _value_with_fmt(self, val) -> tuple[object, str | None]: elif is_bool(val): val = bool(val) elif isinstance(val, datetime.datetime): - fmt = self.datetime_format + fmt = self._datetime_format elif isinstance(val, datetime.date): - fmt = self.date_format + fmt = self._date_format elif isinstance(val, datetime.timedelta): val = val.total_seconds() / 86400 fmt = "0" @@ -1192,6 +1289,16 @@ def check_extension(cls, ext: str) -> Literal[True]: checks that path's extension against the Writer's supported extensions. If it isn't supported, raises UnsupportedFiletypeError. """ + warnings.warn( + "Using check_extension is not part of the public API and will be removed " + "in a future version.", + FutureWarning, + stacklevel=find_stack_level(), + ) + return cls._check_extension(ext) + + @classmethod + def _check_extension(cls, ext: str) -> Literal[True]: if ext.startswith("."): ext = ext[1:] # error: "Callable[[ExcelWriter], Any]" has no attribute "__iter__" (not @@ -1213,8 +1320,8 @@ def __exit__(self, exc_type, exc_value, traceback): def close(self) -> None: """synonym for save, to make it more file-like""" - self.save() - self.handles.close() + self._save() + self._handles.close() XLS_SIGNATURES = ( diff --git a/pandas/io/excel/_odswriter.py b/pandas/io/excel/_odswriter.py index 9069a37ccb5af..f149204ba4967 100644 --- a/pandas/io/excel/_odswriter.py +++ b/pandas/io/excel/_odswriter.py @@ -55,9 +55,17 @@ def __init__( engine_kwargs = combine_kwargs(engine_kwargs, kwargs) - self.book = OpenDocumentSpreadsheet(**engine_kwargs) + self._book = OpenDocumentSpreadsheet(**engine_kwargs) self._style_dict: dict[str, str] = {} + @property + def book(self): + """Book instance of class odf.opendocument.OpenDocumentSpreadsheet. + + This attribute can be used to access engine-specific features. + """ + return self._book + @property def sheets(self) -> dict[str, Any]: """Mapping of sheet names to sheet objects.""" @@ -69,15 +77,15 @@ def sheets(self) -> dict[str, Any]: } return result - def save(self) -> None: + def _save(self) -> None: """ Save workbook to disk. """ for sheet in self.sheets.values(): self.book.spreadsheet.addElement(sheet) - self.book.save(self.handles.handle) + self.book.save(self._handles.handle) - def write_cells( + def _write_cells( self, cells: list[ExcelCell], sheet_name: str | None = None, diff --git a/pandas/io/excel/_openpyxl.py b/pandas/io/excel/_openpyxl.py index 4b03c2536b31b..fa503fa21ff88 100644 --- a/pandas/io/excel/_openpyxl.py +++ b/pandas/io/excel/_openpyxl.py @@ -63,32 +63,40 @@ def __init__( # ExcelWriter replaced "a" by "r+" to allow us to first read the excel file from # the file and later write to it - if "r+" in self.mode: # Load from existing workbook + if "r+" in self._mode: # Load from existing workbook from openpyxl import load_workbook - self.book = load_workbook(self.handles.handle, **engine_kwargs) - self.handles.handle.seek(0) + self._book = load_workbook(self._handles.handle, **engine_kwargs) + self._handles.handle.seek(0) else: # Create workbook object with default optimized_write=True. - self.book = Workbook(**engine_kwargs) + self._book = Workbook(**engine_kwargs) if self.book.worksheets: self.book.remove(self.book.worksheets[0]) + @property + def book(self): + """Book instance of class openpyxl.workbook.Workbook. + + This attribute can be used to access engine-specific features. + """ + return self._book + @property def sheets(self) -> dict[str, Any]: """Mapping of sheet names to sheet objects.""" result = {name: self.book[name] for name in self.book.sheetnames} return result - def save(self) -> None: + def _save(self) -> None: """ Save workbook to disk. """ - self.book.save(self.handles.handle) - if "r+" in self.mode and not isinstance(self.handles.handle, mmap.mmap): + self.book.save(self._handles.handle) + if "r+" in self._mode and not isinstance(self._handles.handle, mmap.mmap): # truncate file to the written content - self.handles.handle.truncate() + self._handles.handle.truncate() @classmethod def _convert_to_style_kwargs(cls, style_dict: dict) -> dict[str, Serialisable]: @@ -424,7 +432,7 @@ def _convert_to_protection(cls, protection_dict): return Protection(**protection_dict) - def write_cells( + def _write_cells( self, cells, sheet_name: str | None = None, @@ -437,23 +445,23 @@ def write_cells( _style_cache: dict[str, dict[str, Serialisable]] = {} - if sheet_name in self.sheets and self.if_sheet_exists != "new": - if "r+" in self.mode: - if self.if_sheet_exists == "replace": + if sheet_name in self.sheets and self._if_sheet_exists != "new": + if "r+" in self._mode: + if self._if_sheet_exists == "replace": old_wks = self.sheets[sheet_name] target_index = self.book.index(old_wks) del self.book[sheet_name] wks = self.book.create_sheet(sheet_name, target_index) - elif self.if_sheet_exists == "error": + elif self._if_sheet_exists == "error": raise ValueError( f"Sheet '{sheet_name}' already exists and " f"if_sheet_exists is set to 'error'." ) - elif self.if_sheet_exists == "overlay": + elif self._if_sheet_exists == "overlay": wks = self.sheets[sheet_name] else: raise ValueError( - f"'{self.if_sheet_exists}' is not valid for if_sheet_exists. " + f"'{self._if_sheet_exists}' is not valid for if_sheet_exists. " "Valid options are 'error', 'new', 'replace' and 'overlay'." ) else: diff --git a/pandas/io/excel/_xlsxwriter.py b/pandas/io/excel/_xlsxwriter.py index dbd6264827591..26247bfad146e 100644 --- a/pandas/io/excel/_xlsxwriter.py +++ b/pandas/io/excel/_xlsxwriter.py @@ -203,20 +203,28 @@ def __init__( engine_kwargs=engine_kwargs, ) - self.book = Workbook(self.handles.handle, **engine_kwargs) + self._book = Workbook(self._handles.handle, **engine_kwargs) + + @property + def book(self): + """Book instance of class xlsxwriter.Workbook. + + This attribute can be used to access engine-specific features. + """ + return self._book @property def sheets(self) -> dict[str, Any]: result = self.book.sheetnames return result - def save(self) -> None: + def _save(self) -> None: """ Save workbook to disk. """ self.book.close() - def write_cells( + def _write_cells( self, cells, sheet_name: str | None = None, diff --git a/pandas/io/excel/_xlwt.py b/pandas/io/excel/_xlwt.py index fe2addc890c22..e4d7dcc108a73 100644 --- a/pandas/io/excel/_xlwt.py +++ b/pandas/io/excel/_xlwt.py @@ -59,9 +59,17 @@ def __init__( if encoding is None: encoding = "ascii" - self.book = xlwt.Workbook(encoding=encoding, **engine_kwargs) - self.fm_datetime = xlwt.easyxf(num_format_str=self.datetime_format) - self.fm_date = xlwt.easyxf(num_format_str=self.date_format) + self._book = xlwt.Workbook(encoding=encoding, **engine_kwargs) + self.fm_datetime = xlwt.easyxf(num_format_str=self._datetime_format) + self.fm_date = xlwt.easyxf(num_format_str=self._date_format) + + @property + def book(self): + """Book instance of class xlwt.Workbook. + + This attribute can be used to access engine-specific features. + """ + return self._book @property def sheets(self) -> dict[str, Any]: @@ -69,15 +77,15 @@ def sheets(self) -> dict[str, Any]: result = {sheet.name: sheet for sheet in self.book._Workbook__worksheets} return result - def save(self) -> None: + def _save(self) -> None: """ Save workbook to disk. """ if self.sheets: # fails when the ExcelWriter is just opened and then closed - self.book.save(self.handles.handle) + self.book.save(self._handles.handle) - def write_cells( + def _write_cells( self, cells, sheet_name: str | None = None, diff --git a/pandas/io/formats/excel.py b/pandas/io/formats/excel.py index 768c462a746dc..2aa67746c16d7 100644 --- a/pandas/io/formats/excel.py +++ b/pandas/io/formats/excel.py @@ -888,7 +888,7 @@ def write( need_save = True try: - writer.write_cells( + writer._write_cells( formatted_cells, sheet_name, startrow=startrow, diff --git a/pandas/tests/io/excel/test_openpyxl.py b/pandas/tests/io/excel/test_openpyxl.py index 0387591a248c1..bab1a1eed97c2 100644 --- a/pandas/tests/io/excel/test_openpyxl.py +++ b/pandas/tests/io/excel/test_openpyxl.py @@ -76,8 +76,8 @@ def test_write_cells_merge_styled(ext): with tm.ensure_clean(ext) as path: with _OpenpyxlWriter(path) as writer: - writer.write_cells(initial_cells, sheet_name=sheet_name) - writer.write_cells(merge_cells, sheet_name=sheet_name) + writer._write_cells(initial_cells, sheet_name=sheet_name) + writer._write_cells(merge_cells, sheet_name=sheet_name) wks = writer.sheets[sheet_name] xcell_b1 = wks["B1"] diff --git a/pandas/tests/io/excel/test_writers.py b/pandas/tests/io/excel/test_writers.py index 4fa9a645c83a2..de4a07e625abe 100644 --- a/pandas/tests/io/excel/test_writers.py +++ b/pandas/tests/io/excel/test_writers.py @@ -1280,10 +1280,13 @@ class DummyClass(ExcelWriter): supported_extensions = ["xlsx", "xls"] engine = "dummy" - def save(self): + def book(self): + pass + + def _save(self): called_save.append(True) - def write_cells(self, *args, **kwargs): + def _write_cells(self, *args, **kwargs): called_write_cells.append(True) @property From 1ab8c23202c97a6d2e1a3c8c3fc3f5d3d0a12971 Mon Sep 17 00:00:00 2001 From: Richard Shadrach Date: Wed, 2 Feb 2022 16:07:57 -0500 Subject: [PATCH 2/5] DEP: Deprecate ExcelWriter attributes --- pandas/tests/io/excel/test_writers.py | 63 ++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/pandas/tests/io/excel/test_writers.py b/pandas/tests/io/excel/test_writers.py index de4a07e625abe..09d215695e0bb 100644 --- a/pandas/tests/io/excel/test_writers.py +++ b/pandas/tests/io/excel/test_writers.py @@ -1245,19 +1245,44 @@ def test_if_sheet_exists_raises(self, ext): ExcelWriter(f, if_sheet_exists="replace") +@pytest.mark.parametrize( + "engine,ext", + [ + pytest.param( + "openpyxl", + ".xlsx", + marks=[td.skip_if_no("openpyxl"), td.skip_if_no("xlrd")], + ), + pytest.param( + "openpyxl", + ".xlsm", + marks=[td.skip_if_no("openpyxl"), td.skip_if_no("xlrd")], + ), + pytest.param( + "xlwt", ".xls", marks=[td.skip_if_no("xlwt"), td.skip_if_no("xlrd")] + ), + pytest.param( + "xlsxwriter", + ".xlsx", + marks=[td.skip_if_no("xlsxwriter"), td.skip_if_no("xlrd")], + ), + pytest.param("odf", ".ods", marks=td.skip_if_no("odf")), + ], +) +@pytest.mark.usefixtures("set_engine") class TestExcelWriterEngineTests: @pytest.mark.parametrize( - "klass,ext", + "klass,klass_ext", [ pytest.param(_XlsxWriter, ".xlsx", marks=td.skip_if_no("xlsxwriter")), pytest.param(_OpenpyxlWriter, ".xlsx", marks=td.skip_if_no("openpyxl")), pytest.param(_XlwtWriter, ".xls", marks=td.skip_if_no("xlwt")), ], ) - def test_ExcelWriter_dispatch(self, klass, ext): - with tm.ensure_clean(ext) as path: + def test_ExcelWriter_dispatch(self, klass, klass_ext): + with tm.ensure_clean(klass_ext) as path: with ExcelWriter(path) as writer: - if ext == ".xlsx" and td.safe_import("xlsxwriter"): + if klass_ext == ".xlsx" and td.safe_import("xlsxwriter"): # xlsxwriter has preference over openpyxl if both installed assert isinstance(writer, _XlsxWriter) else: @@ -1313,14 +1338,6 @@ def check_called(func): with tm.ensure_clean("something.xls") as filepath: check_called(lambda: df.to_excel(filepath, engine="dummy")) - @pytest.mark.parametrize( - "ext", - [ - pytest.param(".xlsx", marks=td.skip_if_no("xlsxwriter")), - pytest.param(".xlsx", marks=td.skip_if_no("openpyxl")), - pytest.param(".ods", marks=td.skip_if_no("odf")), - ], - ) def test_engine_kwargs_and_kwargs_raises(self, ext): # GH 40430 msg = re.escape("Cannot use both engine_kwargs and **kwargs") @@ -1328,6 +1345,28 @@ def test_engine_kwargs_and_kwargs_raises(self, ext): with ExcelWriter("", engine_kwargs={"a": 1}, b=2): pass + @pytest.mark.parametrize("attr", ["cur_sheet", "handles", "path"]) + def test_deprecated_attr(self, engine, ext, attr): + with tm.ensure_clean(ext) as path: + with ExcelWriter(path) as writer: + msg = f"{attr} is not part of the public API" + with tm.assert_produces_warning(FutureWarning, match=msg): + getattr(writer, attr) + # Some engines raise if nothing is written + DataFrame([0]).to_excel(writer) + + @pytest.mark.parametrize( + "attr, args", [("save", ()), ("write_cells", ([], "test"))] + ) + def test_deprecated_method(self, engine, ext, attr, args): + with tm.ensure_clean(ext) as path: + with ExcelWriter(path) as writer: + msg = f"{attr} is not part of the public API" + # Some engines raise if nothing is written + DataFrame([0]).to_excel(writer) + with tm.assert_produces_warning(FutureWarning, match=msg): + getattr(writer, attr)(*args) + @td.skip_if_no("xlrd") @td.skip_if_no("openpyxl") From 567915bae6bfb526b47d991644079ead798c62c6 Mon Sep 17 00:00:00 2001 From: Richard Shadrach Date: Wed, 2 Feb 2022 21:39:09 -0500 Subject: [PATCH 3/5] Fixup for test --- pandas/io/excel/_odswriter.py | 7 ++++--- pandas/tests/io/excel/test_writers.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pandas/io/excel/_odswriter.py b/pandas/io/excel/_odswriter.py index f149204ba4967..5aeea06a1724e 100644 --- a/pandas/io/excel/_odswriter.py +++ b/pandas/io/excel/_odswriter.py @@ -139,9 +139,10 @@ def _write_cells( p = P(text=pvalue) tc.addElement(p) - # add all rows to the sheet - for row_nr in range(max(rows.keys()) + 1): - wks.addElement(rows[row_nr]) + if len(rows) > 0: + # add all rows to the sheet + for row_nr in range(max(rows.keys()) + 1): + wks.addElement(rows[row_nr]) def _make_table_cell_attributes(self, cell) -> dict[str, int | str]: """Convert cell attributes to OpenDocument attributes diff --git a/pandas/tests/io/excel/test_writers.py b/pandas/tests/io/excel/test_writers.py index 09d215695e0bb..a8de60e576f89 100644 --- a/pandas/tests/io/excel/test_writers.py +++ b/pandas/tests/io/excel/test_writers.py @@ -1353,7 +1353,7 @@ def test_deprecated_attr(self, engine, ext, attr): with tm.assert_produces_warning(FutureWarning, match=msg): getattr(writer, attr) # Some engines raise if nothing is written - DataFrame([0]).to_excel(writer) + DataFrame().to_excel(writer) @pytest.mark.parametrize( "attr, args", [("save", ()), ("write_cells", ([], "test"))] @@ -1363,7 +1363,7 @@ def test_deprecated_method(self, engine, ext, attr, args): with ExcelWriter(path) as writer: msg = f"{attr} is not part of the public API" # Some engines raise if nothing is written - DataFrame([0]).to_excel(writer) + DataFrame().to_excel(writer) with tm.assert_produces_warning(FutureWarning, match=msg): getattr(writer, attr)(*args) From bcad3e88600f5a2d2e1f746e9cf339ffa376ec40 Mon Sep 17 00:00:00 2001 From: Richard Shadrach Date: Wed, 2 Feb 2022 22:28:50 -0500 Subject: [PATCH 4/5] Move tests and restore check_extension y --- pandas/io/excel/_base.py | 12 +--- pandas/tests/io/excel/test_writers.py | 87 +++++++++++---------------- 2 files changed, 37 insertions(+), 62 deletions(-) diff --git a/pandas/io/excel/_base.py b/pandas/io/excel/_base.py index a88fab48eed5e..5b331707b9dc2 100644 --- a/pandas/io/excel/_base.py +++ b/pandas/io/excel/_base.py @@ -1136,7 +1136,7 @@ def __init__( # validate that this engine can handle the extension if isinstance(path, str): ext = os.path.splitext(path)[-1] - self._check_extension(ext) + self.check_extension(ext) # use mode to open the file if "b" not in mode: @@ -1289,16 +1289,6 @@ def check_extension(cls, ext: str) -> Literal[True]: checks that path's extension against the Writer's supported extensions. If it isn't supported, raises UnsupportedFiletypeError. """ - warnings.warn( - "Using check_extension is not part of the public API and will be removed " - "in a future version.", - FutureWarning, - stacklevel=find_stack_level(), - ) - return cls._check_extension(ext) - - @classmethod - def _check_extension(cls, ext: str) -> Literal[True]: if ext.startswith("."): ext = ext[1:] # error: "Callable[[ExcelWriter], Any]" has no attribute "__iter__" (not diff --git a/pandas/tests/io/excel/test_writers.py b/pandas/tests/io/excel/test_writers.py index a8de60e576f89..ff915557602dd 100644 --- a/pandas/tests/io/excel/test_writers.py +++ b/pandas/tests/io/excel/test_writers.py @@ -1244,45 +1244,44 @@ def test_if_sheet_exists_raises(self, ext): with pytest.raises(ValueError, match=re.escape(msg)): ExcelWriter(f, if_sheet_exists="replace") + @pytest.mark.parametrize("attr", ["cur_sheet", "handles", "path"]) + def test_deprecated_attr(self, engine, ext, attr): + # GH#45572 + with tm.ensure_clean(ext) as path: + with ExcelWriter(path) as writer: + msg = f"{attr} is not part of the public API" + with tm.assert_produces_warning(FutureWarning, match=msg): + getattr(writer, attr) + # Some engines raise if nothing is written + DataFrame().to_excel(writer) + + @pytest.mark.parametrize( + "attr, args", [("save", ()), ("write_cells", ([], "test"))] + ) + def test_deprecated_method(self, engine, ext, attr, args): + # GH#45572 + with tm.ensure_clean(ext) as path: + with ExcelWriter(path) as writer: + msg = f"{attr} is not part of the public API" + # Some engines raise if nothing is written + DataFrame().to_excel(writer) + with tm.assert_produces_warning(FutureWarning, match=msg): + getattr(writer, attr)(*args) + -@pytest.mark.parametrize( - "engine,ext", - [ - pytest.param( - "openpyxl", - ".xlsx", - marks=[td.skip_if_no("openpyxl"), td.skip_if_no("xlrd")], - ), - pytest.param( - "openpyxl", - ".xlsm", - marks=[td.skip_if_no("openpyxl"), td.skip_if_no("xlrd")], - ), - pytest.param( - "xlwt", ".xls", marks=[td.skip_if_no("xlwt"), td.skip_if_no("xlrd")] - ), - pytest.param( - "xlsxwriter", - ".xlsx", - marks=[td.skip_if_no("xlsxwriter"), td.skip_if_no("xlrd")], - ), - pytest.param("odf", ".ods", marks=td.skip_if_no("odf")), - ], -) -@pytest.mark.usefixtures("set_engine") class TestExcelWriterEngineTests: @pytest.mark.parametrize( - "klass,klass_ext", + "klass,ext", [ pytest.param(_XlsxWriter, ".xlsx", marks=td.skip_if_no("xlsxwriter")), pytest.param(_OpenpyxlWriter, ".xlsx", marks=td.skip_if_no("openpyxl")), pytest.param(_XlwtWriter, ".xls", marks=td.skip_if_no("xlwt")), ], ) - def test_ExcelWriter_dispatch(self, klass, klass_ext): - with tm.ensure_clean(klass_ext) as path: + def test_ExcelWriter_dispatch(self, klass, ext): + with tm.ensure_clean(ext) as path: with ExcelWriter(path) as writer: - if klass_ext == ".xlsx" and td.safe_import("xlsxwriter"): + if ext == ".xlsx" and td.safe_import("xlsxwriter"): # xlsxwriter has preference over openpyxl if both installed assert isinstance(writer, _XlsxWriter) else: @@ -1338,6 +1337,14 @@ def check_called(func): with tm.ensure_clean("something.xls") as filepath: check_called(lambda: df.to_excel(filepath, engine="dummy")) + @pytest.mark.parametrize( + "ext", + [ + pytest.param(".xlsx", marks=td.skip_if_no("xlsxwriter")), + pytest.param(".xlsx", marks=td.skip_if_no("openpyxl")), + pytest.param(".ods", marks=td.skip_if_no("odf")), + ], + ) def test_engine_kwargs_and_kwargs_raises(self, ext): # GH 40430 msg = re.escape("Cannot use both engine_kwargs and **kwargs") @@ -1345,28 +1352,6 @@ def test_engine_kwargs_and_kwargs_raises(self, ext): with ExcelWriter("", engine_kwargs={"a": 1}, b=2): pass - @pytest.mark.parametrize("attr", ["cur_sheet", "handles", "path"]) - def test_deprecated_attr(self, engine, ext, attr): - with tm.ensure_clean(ext) as path: - with ExcelWriter(path) as writer: - msg = f"{attr} is not part of the public API" - with tm.assert_produces_warning(FutureWarning, match=msg): - getattr(writer, attr) - # Some engines raise if nothing is written - DataFrame().to_excel(writer) - - @pytest.mark.parametrize( - "attr, args", [("save", ()), ("write_cells", ([], "test"))] - ) - def test_deprecated_method(self, engine, ext, attr, args): - with tm.ensure_clean(ext) as path: - with ExcelWriter(path) as writer: - msg = f"{attr} is not part of the public API" - # Some engines raise if nothing is written - DataFrame().to_excel(writer) - with tm.assert_produces_warning(FutureWarning, match=msg): - getattr(writer, attr)(*args) - @td.skip_if_no("xlrd") @td.skip_if_no("openpyxl") From eb0d9d1187f60470fd527464497a6350ece32345 Mon Sep 17 00:00:00 2001 From: Richard Shadrach Date: Sat, 5 Feb 2022 15:34:27 -0500 Subject: [PATCH 5/5] Deprecate xlwt fm_date and fm_datetime; doc improvements --- doc/source/whatsnew/v1.5.0.rst | 36 +++++++++++++++++++++++++----- pandas/io/excel/_base.py | 3 ++- pandas/io/excel/_odswriter.py | 3 ++- pandas/io/excel/_openpyxl.py | 3 ++- pandas/io/excel/_xlsxwriter.py | 3 ++- pandas/io/excel/_xlwt.py | 23 ++++++++++++++++--- pandas/tests/io/excel/test_xlwt.py | 10 +++++++++ 7 files changed, 68 insertions(+), 13 deletions(-) diff --git a/doc/source/whatsnew/v1.5.0.rst b/doc/source/whatsnew/v1.5.0.rst index a7bddaa85a155..73eee5885e364 100644 --- a/doc/source/whatsnew/v1.5.0.rst +++ b/doc/source/whatsnew/v1.5.0.rst @@ -189,13 +189,37 @@ Slicing on a :class:`DataFrame` will not be affected. :class:`ExcelWriter` attributes ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -The attributes and methods of :class:`ExcelWriter` were previously documented as not -being public. However some third party Excel engines documented accessing +All attributes of :class:`ExcelWriter` were previously documented as not +public. However some third party Excel engines documented accessing ``ExcelWriter.book`` or ``ExcelWriter.sheets``, and users were utilizing these -and possibly other attributes. In order to support this, pandas has made some -attributes and methods public. Other attributes and methods have been deprecated -and will raise a ``FutureWarning`` when accessed as they will be removed in a future -version. See the documentation of :class:`ExcelWriter` for further details. +and possibly other attributes. Previously these attributes were not safe to use; +e.g. modifications to ``ExcelWriter.book`` would not update ``ExcelWriter.sheets`` +and conversely. In order to support this, pandas has made some attributes public +and improved their implementations so that they may now be safely used. (:issue:`45572`) + +The following attributes are now public and considered safe to access. + + - ``book`` + - ``check_extension`` + - ``close`` + - ``date_format`` + - ``datetime_format`` + - ``engine`` + - ``if_sheet_exists`` + - ``sheets`` + - ``supported_extensions`` + +The following attributes have been deprecated. They now raise a ``FutureWarning`` +when accessed and will removed in a future version. Users should be aware +that their usage is considered unsafe, and can lead to unexpected results. + + - ``cur_sheet`` + - ``handles`` + - ``path`` + - ``save`` + - ``write_cells`` + +See the documentation of :class:`ExcelWriter` for further details. .. _whatsnew_150.deprecations.other: diff --git a/pandas/io/excel/_base.py b/pandas/io/excel/_base.py index 5b331707b9dc2..c32f84d41ebde 100644 --- a/pandas/io/excel/_base.py +++ b/pandas/io/excel/_base.py @@ -1047,7 +1047,8 @@ def sheets(self) -> dict[str, Any]: @property @abc.abstractmethod def book(self): - """Book instance. Class type will depend on the engine used. + """ + Book instance. Class type will depend on the engine used. This attribute can be used to access engine-specific features. """ diff --git a/pandas/io/excel/_odswriter.py b/pandas/io/excel/_odswriter.py index e2a2250a757e8..94f173c1469e0 100644 --- a/pandas/io/excel/_odswriter.py +++ b/pandas/io/excel/_odswriter.py @@ -60,7 +60,8 @@ def __init__( @property def book(self): - """Book instance of class odf.opendocument.OpenDocumentSpreadsheet. + """ + Book instance of class odf.opendocument.OpenDocumentSpreadsheet. This attribute can be used to access engine-specific features. """ diff --git a/pandas/io/excel/_openpyxl.py b/pandas/io/excel/_openpyxl.py index fa503fa21ff88..1ef4c348773bc 100644 --- a/pandas/io/excel/_openpyxl.py +++ b/pandas/io/excel/_openpyxl.py @@ -77,7 +77,8 @@ def __init__( @property def book(self): - """Book instance of class openpyxl.workbook.Workbook. + """ + Book instance of class openpyxl.workbook.Workbook. This attribute can be used to access engine-specific features. """ diff --git a/pandas/io/excel/_xlsxwriter.py b/pandas/io/excel/_xlsxwriter.py index 26247bfad146e..45fe4713ce194 100644 --- a/pandas/io/excel/_xlsxwriter.py +++ b/pandas/io/excel/_xlsxwriter.py @@ -207,7 +207,8 @@ def __init__( @property def book(self): - """Book instance of class xlsxwriter.Workbook. + """ + Book instance of class xlsxwriter.Workbook. This attribute can be used to access engine-specific features. """ diff --git a/pandas/io/excel/_xlwt.py b/pandas/io/excel/_xlwt.py index e4d7dcc108a73..871fcbd3a8475 100644 --- a/pandas/io/excel/_xlwt.py +++ b/pandas/io/excel/_xlwt.py @@ -60,12 +60,13 @@ def __init__( if encoding is None: encoding = "ascii" self._book = xlwt.Workbook(encoding=encoding, **engine_kwargs) - self.fm_datetime = xlwt.easyxf(num_format_str=self._datetime_format) - self.fm_date = xlwt.easyxf(num_format_str=self._date_format) + self._fm_datetime = xlwt.easyxf(num_format_str=self._datetime_format) + self._fm_date = xlwt.easyxf(num_format_str=self._date_format) @property def book(self): - """Book instance of class xlwt.Workbook. + """ + Book instance of class xlwt.Workbook. This attribute can be used to access engine-specific features. """ @@ -77,6 +78,22 @@ def sheets(self) -> dict[str, Any]: result = {sheet.name: sheet for sheet in self.book._Workbook__worksheets} return result + @property + def fm_date(self): + """ + XFStyle formatter for dates. + """ + self._deprecate("fm_date") + return self._fm_date + + @property + def fm_datetime(self): + """ + XFStyle formatter for dates. + """ + self._deprecate("fm_datetime") + return self._fm_datetime + def _save(self) -> None: """ Save workbook to disk. diff --git a/pandas/tests/io/excel/test_xlwt.py b/pandas/tests/io/excel/test_xlwt.py index 2d5386d6c616d..3aa405eb1e275 100644 --- a/pandas/tests/io/excel/test_xlwt.py +++ b/pandas/tests/io/excel/test_xlwt.py @@ -134,3 +134,13 @@ def test_book_and_sheets_consistent(ext): assert writer.sheets == {} sheet = writer.book.add_sheet("test_name") assert writer.sheets == {"test_name": sheet} + + +@pytest.mark.parametrize("attr", ["fm_date", "fm_datetime"]) +def test_deprecated_attr(ext, attr): + # GH#45572 + with tm.ensure_clean(ext) as path: + with ExcelWriter(path, engine="xlwt") as writer: + msg = f"{attr} is not part of the public API" + with tm.assert_produces_warning(FutureWarning, match=msg): + getattr(writer, attr)