Skip to content

DEP: Remove xlrd as being the default reader for xlsx #39796

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

Merged
merged 5 commits into from
Feb 16, 2021

Conversation

rhshadrach
Copy link
Member

One thing I noticed while working on this is that xlrd is in requirements-dev but not environment.yml; not sure if it should be removed from requirements-dev.

cc @jreback @jorisvandenbossche

@rhshadrach rhshadrach added IO Excel read_excel, to_excel Dependencies Required and optional dependencies labels Feb 13, 2021
@jreback jreback added this to the 1.3 milestone Feb 15, 2021
@jreback
Copy link
Contributor

jreback commented Feb 15, 2021

dont' we have any tests that needed to be changed? (also pls merge master)

@jreback
Copy link
Contributor

jreback commented Feb 15, 2021

One thing I noticed while working on this is that xlrd is in requirements-dev but not environment.yml; not sure if it should be removed from requirements-dev.

requirements-dev is autogenerated from environment.yml so this would be strange (but it appears to be in both); maybe we should sort requirements-dev.txt ?

@WillAyd
Copy link
Member

WillAyd commented Feb 16, 2021

Can we add a test that openpyxl is actually being dispatched to as the default?

@rhshadrach
Copy link
Member Author

@jreback

requirements-dev is autogenerated from environment.yml so this would be strange (but it appears to be in both)

Whoops - not sure what I was looking at then :)

@jreback, @WillAyd - tests added. For ExcelFile we can get the engine directly, for pd.read_excel I monkey patched ExcelFile.parser.

@jreback jreback merged commit 7829f05 into pandas-dev:master Feb 16, 2021
@jreback
Copy link
Contributor

jreback commented Feb 16, 2021

very nice @rhshadrach

@rhshadrach rhshadrach deleted the xlsx_default branch February 16, 2021 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Required and optional dependencies IO Excel read_excel, to_excel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DEPR: Remove xlrd as being the default reader for xlsx
3 participants