Skip to content
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

Deprecate xlwt #26552

Closed
WillAyd opened this issue May 29, 2019 · 9 comments · Fixed by #38317
Closed

Deprecate xlwt #26552

WillAyd opened this issue May 29, 2019 · 9 comments · Fixed by #38317
Assignees
Labels
Deprecate Functionality to remove in pandas IO Excel read_excel, to_excel
Milestone

Comments

@WillAyd
Copy link
Member

WillAyd commented May 29, 2019

Similar to the conversation in #26487 xlwt isn't maintained. If we deprecate this could open up the path to simplify our Excel writers / extensions and supported file types going forward.

I would think this is also much easier than deprecating xlrd since we have other writers already implemented.

@WillAyd WillAyd added Deprecate Functionality to remove in pandas IO Excel read_excel, to_excel labels May 29, 2019
@WillAyd WillAyd added this to the Contributions Welcome milestone May 29, 2019
@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented May 29, 2019

That would also mean deprecating the xls format? (or can the other engines write such files as well?)

I don't think I necessarily mind (for writing, as long as we can still read old formats), but just to make it clear.

@WillAyd
Copy link
Member Author

WillAyd commented May 29, 2019

That's correct - xlwt and xlrd are the only engines that support the .xls format

@jbrockmendel
Copy link
Member

Is there any value in getting this (and the other excel-related deprecations) done for 1.0? If we're not enforcing the deprecations until 2.0, im tempted to just ignore all DEPR issues until [2 releases before 2.0]

@WillAyd
Copy link
Member Author

WillAyd commented Dec 11, 2019

Probably worth getting #29375 in before 1.0

@jreback
Copy link
Contributor

jreback commented Dec 1, 2020

cc @pandas-dev/pandas-core if anyone wants to do this for 1.2

@rhshadrach
Copy link
Member

@jreback Is this something we could potentially do during the rc phase for 1.2? I can take this up, but not for another 2 days.

@jreback
Copy link
Contributor

jreback commented Dec 4, 2020

yeah i think this would be fine during rc

@rhshadrach rhshadrach self-assigned this Dec 4, 2020
@rhshadrach
Copy link
Member

Based on the decisions made in #35029, I'm not sure there is anything to do here. For reading, when a user specifies engine="xlrd" or the extension is .xls, the xlrd engine is used and no warning is emitted. I think we want to parallel that behavior for writing, and as far as I can tell, that is already the case.

Namely, we get the config f"io.excel.{ext}.writer" where ext is the file extension (defaults to "xlsx" if path is not a string). This config defaults to "auto" which uses the following logic to determine the engine based on the extension.

    _default_writers = {
        "xlsx": "openpyxl",
        "xlsm": "openpyxl",
        "xls": "xlwt",
        "ods": "odf",
    }
    xlsxwriter = import_optional_dependency(
        "xlsxwriter", raise_on_missing=False, on_version="warn"
    )
    if xlsxwriter:
        _default_writers["xlsx"] = "xlsxwriter"

So a user will only be using xlwt if they specify engine="xlwt", have overridden the default config, or if the path is a string with the extension xls. I think in each of these cases, we do not want to emit a warning.

@rhshadrach
Copy link
Member

Ahh, I see. I think we want to deprecate writing xls but not reading xls, so a DeprecationWarning should be emitted anytime xlwt is used regardless of how we got there.

@jreback jreback modified the milestones: Contributions Welcome, 1.2 Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas IO Excel read_excel, to_excel
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants