Added a decorator function to validate parameters

[shantanuo]

read_excel method needs to check if it has received valid parameters only. Use of **kwds allows any parameter that leads to confusion and bugs.

[pep8speaks]

Hello @shantanuo! Thanks for updating the PR.

• In the file pandas/io/excel.py, following are the PEP8 issues :

Line 282:1: E302 expected 2 blank lines, found 1

• In the file pandas/util/_decorators.py, following are the PEP8 issues :

Line 8:1: E302 expected 2 blank lines, found 1
Line 12:26: E225 missing whitespace around operator
Line 12:50: E231 missing whitespace after ','
Line 12:80: E501 line too long (122 > 79 characters)
Line 12:123: W291 trailing whitespace
Line 13:16: E128 continuation line under-indented for visual indent
Line 13:44: E241 multiple spaces after ','
Line 13:80: E501 line too long (120 > 79 characters)
Line 14:16: E128 continuation line under-indented for visual indent
Line 14:80: E501 line too long (85 > 79 characters)
Line 16:61: W291 trailing whitespace

Comment last updated on August 04, 2018 at 03:46 Hours UTC

[jbrockmendel]

Take a look at pandas.util._validators.validate_kwargs

[WillAyd]

First and foremost this needs tests.

If this is only for read_excel then I think the decorator is overkill - would be better served directly within the function body.

Is this in reference a particular issue #? If so can you update your post to link to it?

[shantanuo]

This is not only related to "read_excel" function. All read_* methods accept *kwargs due to which non-valid parameters are allowed.

#22189

This is the only issue that is holding me back from using pandas and therefore I tried to fix it :)

[WillAyd]

I still think the wrapper is overcomplicating things. As mentioned before tests are the most critical thing here and ideally should come before any other coding changes

[WillAyd]

Just instantiate this as a set

[WillAyd]

Would be nice to be explicit about which parameter isn't working. Also need a test

[TomAugspurger]

I think we should focus our efforts on removing the need for **kwargs from the signature, rather than trying to work around it.

[shantanuo]

sure. Decorator was my shortcut to incorporate the functionality that I needed desperately :)

[jbrockmendel]

Am I missing something? Why is the existing validate_kwargs decorator not up to the task?

[shantanuo]

I checked the existing validate_kwargs decorator, but could not figure out how to use it :)
Removing **kwargs seems to be ideal though. Any one of the two (that is practical, easy and quick) approach should be adopted.

[jbrockmendel]

@shantanuo any plans to add a test to this? I doubt this implementation will be merged, but a test could help us figure out how to use the existing decorators for this purpose.

[jreback]

closing as stale. if you'd like to continue, pls ping.