-
-
Notifications
You must be signed in to change notification settings - Fork 157
TYP: enhance ExcelWriter with generic type support and overloads
#1561
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
base: main
Are you sure you want to change the base?
Conversation
9en9i
commented
Dec 17, 2025
- Closes Generic ExcelWriter #1559
- Tests for ExcelWriter fixed
ExcelWriter with generic type support and overloadsExcelWriter with generic type support and overloads
| @overload | ||
| def __init__( | ||
| self, | ||
| self: ExcelWriter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
| self: ExcelWriter, | |
| self, |
will do
| def __init__( | ||
| self, | ||
| self: ExcelWriter, | ||
| path: FilePath | WriteExcelBuffer | ExcelWriter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the docs only FilePath and BinaryIO are supported
| path: FilePath | WriteExcelBuffer | ExcelWriter, | |
| path: FilePath | BinaryIO, |
Also to other overloads
| engine: Literal["auto"] | None = ..., | ||
| date_format: str | None = ..., | ||
| datetime_format: str | None = ..., | ||
| mode: Literal["w", "a"] = ..., | ||
| storage_options: StorageOptions = ..., | ||
| if_sheet_exists: ExcelWriterIfSheetExists | None = ..., | ||
| engine_kwargs: dict[str, Any] | None = ..., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add default values
| engine: Literal["auto"] | None = ..., | |
| date_format: str | None = ..., | |
| datetime_format: str | None = ..., | |
| mode: Literal["w", "a"] = ..., | |
| storage_options: StorageOptions = ..., | |
| if_sheet_exists: ExcelWriterIfSheetExists | None = ..., | |
| engine_kwargs: dict[str, Any] | None = ..., | |
| engine: Literal["auto"] | None = None, | |
| date_format: str | None = None, | |
| datetime_format: str | None = None, | |
| mode: Literal["w", "a"] = "w", | |
| storage_options: StorageOptions = None, | |
| if_sheet_exists: ExcelWriterIfSheetExists = "error", | |
| engine_kwargs: Mapping[str, Any] | None = None, |
Also to other overloads
| def __init__( | ||
| self: ExcelWriter[Workbook], | ||
| path: FilePath | WriteExcelBuffer | ExcelWriter[Workbook], | ||
| engine: Literal["openpyxl"] = ..., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| engine: Literal["openpyxl"] = ..., | |
| engine: Literal["openpyxl"], |
it has to be the fixed literal openpyxl, if I'm not mistaken
| def __init__( | ||
| self: ExcelWriter[OpenDocument], | ||
| path: FilePath | WriteExcelBuffer | ExcelWriter[OpenDocument], | ||
| engine: Literal["odf"] = ..., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| engine: Literal["odf"] = ..., | |
| engine: Literal["odf"], |
| def __init__( | ||
| self: ExcelWriter[XlsxWorkbook], | ||
| path: FilePath | WriteExcelBuffer | ExcelWriter[XlsxWorkbook], | ||
| engine: Literal["xlsxwriter"] = ..., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| engine: Literal["xlsxwriter"] = ..., | |
| engine: Literal["xlsxwriter"], |
|
|
||
| class ExcelWriter(Generic[_WorkbookT]): | ||
| @overload | ||
| def __init__( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am confused, maybe we should overload __new__ instead of __init__ here? cc @Dr-Irv @loicdiridollou