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

read_excel does not raise error with wrong keyword arg #17994

Closed
lucianoviola opened this issue Oct 26, 2017 · 10 comments

Comments

Projects
None yet
5 participants
@lucianoviola
Copy link
Contributor

commented Oct 26, 2017

For example:

pd.read_excel('file_name.xlsx',sheet='mysheet')

does not raise any error. The right parameter name is "sheetname" . If "sheet" is passed it defaults to the first sheet and no error is raised.

sheetname and sheet are very similar and easy to confuse.

One might think that it is reading the sheet passed as argument when in fact it is another one.

I would like to suggest for errors to be raised if unexpected args are passed.

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2017

This may be little tricky, since IIRC we pass through **kwargs to the underlying engine doing the work (though I may be wrong). Can you take a look in pandas/io/excel.py to see if there's anything we can do?

@gfyoung gfyoung added the IO Excel label Oct 26, 2017

@TomAugspurger TomAugspurger added this to the Next Major Release milestone Oct 26, 2017

@gfyoung

This comment has been minimized.

Copy link
Member

commented Oct 26, 2017

@TomAugspurger : You are correct regarding **kwargs. That is indeed quite unfortunate.

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2017

Hmm, we could maybe explicitly check for some common mistakes (like sheet), verify that none of the engines use them, and raise if we see them? That's fragile to the engines adding them in the future :/

@gfyoung

This comment has been minimized.

Copy link
Member

commented Oct 26, 2017

@TomAugspurger : I think we should look into expanding the signature to obviate the need for **kwargs. Ultimately, we would like it to be identical to read_csv, but that's another story for another day.

@lucianoviola

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2017

@TomAugspurger: I’m not an experienced programmer, but I could try something. What about something as simple as if parameter passed not in a list of allowed parameters, raise an error?

@gfyoung

This comment has been minimized.

Copy link
Member

commented Oct 30, 2017

@lucianoviola : That sounds like a good plan. Refer to read_csv to see which arguments we can accept. Also, use the same error message that you get when you pass in an unexpected argument to a function (i.e. "[function] got an unexpected keyword argument [arg]")

@lucianoviola

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2017

@gfyoung Ok!

@joonro

This comment has been minimized.

Copy link

commented Dec 5, 2017

+1. I just got bitten by sheet_name.

@lucianoviola

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2018

Hey guys, I'm having a bit of difficulty figuring out how to solve this one. I'm new to open source contributing, but I'm really excited about it.

My initial solution was to raise a warning if "sheet" was in kwargs, it does not generalize to things like "Sheet".

So, in my view, we either list all possible misspellings and and confusion possibilities of sheetname and check directly check it they where passed, or we review the need to have kwargs.

I don't think that words like "Sheet' are such a problem, I don't think its a very common mistake. The one I'm concerned about is "sheet".

What do you guys think? =)

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented May 26, 2018

@jreback jreback modified the milestones: Contributions Welcome, 0.24.0 Jul 7, 2018

jreback added a commit that referenced this issue Jul 7, 2018

ENH: Raise error for 'sheet' arg in read_excel (#18604)
* ENH: Raise error for read_excel possible "sheet" argument in kwargs (#17994)

alimcmaster1 added a commit to alimcmaster1/pandas that referenced this issue Aug 12, 2018

ENH: Raise error for 'sheet' arg in read_excel (pandas-dev#18604)
* ENH: Raise error for read_excel possible "sheet" argument in kwargs (pandas-dev#17994)

Sup3rGeo added a commit to Sup3rGeo/pandas that referenced this issue Oct 1, 2018

ENH: Raise error for 'sheet' arg in read_excel (pandas-dev#18604)
* ENH: Raise error for read_excel possible "sheet" argument in kwargs (pandas-dev#17994)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.