Skip to content

EHN: Refactor pd.read_excel/DataFrame.to_excel like DataFrame.plot #40291

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

Closed
rhshadrach opened this issue Mar 7, 2021 · 3 comments
Closed

EHN: Refactor pd.read_excel/DataFrame.to_excel like DataFrame.plot #40291

rhshadrach opened this issue Mar 7, 2021 · 3 comments
Labels
API Design Enhancement IO Excel read_excel, to_excel Needs Discussion Requires discussion from core team before further action

Comments

@rhshadrach
Copy link
Member

rhshadrach commented Mar 7, 2021

Two issues have recently come up (#40274 and #40230) looking to add engine-specific arguments to either pd.read_excel or DataFrame.to_excel. Thus far these are either just to pass through the engine or fix/enhance the current pandas layer, and I'm generally positive on doing so, but I do not like adding engine-specific arguments to these methods.

Thus I am proposing to refactor pd.read_excel and DataFrame.to_excel to mirror (at least, from an API perspective) the design of DataFrame.plot so that a user could call, e.g.

  • pd.read_excel
  • pd.read_excel.openpyxl
  • pd.read_excel.xlrd
  • pd.read_excel.ods
  • etc.

The arguments to pd.read_excel would be those that are common to all engines, whereas the arguments to e.g. pd.read_excel.openpyxl would be that plus any engine-specific arguments. The new methods would just be interfaces to the backend subclasses that exist today, and only responsible for validating the arguments provided.

The only alternative I see (besides not allowing engine-specific options at all, which is also an option) is to use **kwargs instead. This would result in poorer documentation and we would need to react to upstream enhancements in the engine rather than being able opt-in to them.

@rhshadrach rhshadrach added Enhancement API Design IO Excel read_excel, to_excel Needs Discussion Requires discussion from core team before further action labels Mar 7, 2021
@jreback
Copy link
Contributor

jreback commented Mar 7, 2021

-1 on this

would be ok with having an engine_kwargs arg instead as we do elsewhere

@jreback
Copy link
Contributor

jreback commented Mar 7, 2021

also a little leary of both of the references issues esp on anything involving xlrd

@rhshadrach
Copy link
Member Author

Thanks @jreback - makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Enhancement IO Excel read_excel, to_excel Needs Discussion Requires discussion from core team before further action
Projects
None yet
Development

No branches or pull requests

2 participants