CLN GH10559 Rename sheetname variable to sheet_name for consistency #12604
Conversation
|
reason you closed this? |
|
Hi jreback, I did not mean to close the pull request. This is my first attempt at contributing and I'm going through some growing pains. I realized as soon as I submitted the request that I had accidentally changed the keyword arguments instead of keeping them as sheetname. I will make another attempt at fixing the issue if that's okay. |
|
contributing docs are here |
| @@ -72,7 +72,7 @@ def get_writer(engine_name): | |||
| raise ValueError("No Excel writer '%s'" % engine_name) | |||
|
|
|||
|
|
|||
| def read_excel(io, sheetname=0, header=0, skiprows=None, skip_footer=0, | |||
| def read_excel(io, sheet_name=0, header=0, skiprows=None, skip_footer=0, | |||
There was a problem hiding this comment.
so this will actually need to be deprecated, so re-add sheetname=None. Then if sheetname is not None, you will issue a warning that it is deprecated and reset sheet_name=sheetname.
This way if people pass as positional args it will work, and if they pass as a kwarg then it will show the deprecation warning (but still work).
Pls add a tests for this as well.
|
pls reopen if you like to rebase / update |
|
Hi @jreback, sorry for going M.I.A. on this. I want to fix this issue, but I have a few questions. When you say sheetname=None, do you mean you want to change sheetname=0 to sheetname=None? If this is the case, my plan is to change these arguments in read_excel() and _parse_excel() while also adding a deprecation warning. Please let me know your thoughts. |
|
so you rename the arg as you did, then add the deprecated kwarg at the end, e.g. |
|
@jreback Hi cloned the fork and created a dev environment by using python setup.py develop. When I made the necessary changes, I noticed that all of the tests still passed when they shouldn't have. I read the docs again to see if I missed anything, and asked on IRC, but we couldn't come up with a solution. Is there something I need to be doing with nosetests in order to run a similar test suite to the one on Travis? Thanks, |
git diff upstream/master | flake8 --diff