ENH: Allow safe access to .book in ExcelWriter
#45687
Conversation
|
Failures look related? |
|
Thanks @phofl - wasn't using a context manager for ExcelWriter |
|
Failure is unrelated. |
|
You did not add a test for xlsxwriter, was this on purpose? |
Yes, but I realize now that was a mistake. I removed the |
| @@ -58,6 +58,16 @@ def __init__( | |||
| self.book = OpenDocumentSpreadsheet(**engine_kwargs) | |||
| self._style_dict: dict[str, str] = {} | |||
|
|
|||
| @property | |||
| def sheets(self) -> dict[str, Any]: | |||
There was a problem hiding this comment.
is there a way to type these more specifically?
There was a problem hiding this comment.
Not easily, as far as I can tell. I plan to look into this more in the future.
Regarding whatsnew - this is not user facing. Currently in docs no attribute of ExcelWriter is public. I'd like to refactor ExcelWriter (namely, making protected attributes start with _) before updating docs.
There was a problem hiding this comment.
got it totally fine (make issues if you cannot to get to it soon), otherwise PR's totally fine.
is there a way to type these more specifically?
can we use the base class generic for typing? (again nbd)
|
@jreback - added docstrings. I'd like to hold off updating the docs until the API is ready to be public. |
| if sheet_name in self.sheets: | ||
| wks = self.sheets[sheet_name] | ||
| else: | ||
| wks = self.book.get_worksheet_by_name(sheet_name) |
There was a problem hiding this comment.
i would change .book to be a private attribute ._book and then expose the .book property (followups for sure)
There was a problem hiding this comment.
Would the property be something besides just return self._book? In other words, why a property?
Towards #45572
The plan to expose both
sheetsandbookas public attributes ofExcelWriter. We need to make sure thatsheetsis correct if the user modifies the state via accessingbook.This accesses a protected attribute for xlwt; I looked at the xlwt code and there is no public access to determine the sheets. The xlwt repository is read-only, we only support the most recent version, and xlwt will be removed in pandas 2.0. So I think this is okay.