CLN:Remove unused **kwargs from user facing methods

[kprestel]

closes GH18748 for user facing methods.

An updated extra_kwargs.txt mostly shows internal functions and false positives.

There are a few cases like all of the visit_* functions that removing the **kwargs breaks the API.

• xref Silently Ignored Kwargs Cleanup #18748
• [x ] tests added / passed
• [x ] passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry - N/A only code cleanup.

[pep8speaks]

Hello @kprestel! Thanks for updating the PR.

• There are no PEP8 issues in the file pandas/io/pytables.py !

Comment last updated on November 11, 2018 at 16:41 Hours UTC

[codecov]

Codecov Report

Merging #23249 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #23249   +/-   ##
=======================================
  Coverage   92.23%   92.23%           
=======================================
  Files         161      161           
  Lines       51288    51288           
=======================================
  Hits        47304    47304           
  Misses       3984     3984

Flag	Coverage Δ
#multiple	90.62% <100%> (ø)	⬆️
#single	42.26% <100%> (ø)	⬆️

Impacted Files	Coverage Δ
pandas/io/pytables.py	92.34% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58a59bd...0a785d5. Read the comment docs.

[WillAyd]

Quick glance at the CI failures seem related to the change. If you could take a look would be great, ping if you need help

[kprestel]

@WillAyd I think I fixed the tests (they all pass locally) but the change I had to make seems a bit counter to the essence of this issue.

It seems that the failures were mostly related to passing the kwarg errors added in regards to issue #20835, but naively I seems that it doesn't actually do anything. It just seems like its forcing **kwargs to be passed around to several functions but it only contains errors which from what I can tell doesn't do anything.

Its likely that I just don't know what I'm talking about and this is needed for something that I just don't know about, but I wanted to at least bring it up so someone with more knowledge of the code base can look a bit closer at it.

Thanks again for the review and the help!

[WillAyd]

Hmm OK. I personally wouldn't be opposed to making errors a named argument if it helps with the code readability - how many functions are we talking about?

[kprestel]

@WillAyd It wasn't a ton of functions. Maybe 3-4?

[WillAyd]

I'm not against using an explicit named parameter instead of kwargs for the errors. That would also somewhat progress towards #20903 so if it is simple and helps code readability I say go for that

[jreback]

does this competely close #18748 ?

[jreback]

can you merge master and update

[kprestel]

@jreback Addressed your request for changes and yes this completely closes #18748. There are some internal functions as well as test functions that still have this issue but as you said in the issue, those aren't really an issue. I have run the script from the issue and the results are posted in this PR.

@WillAyd I don't actually think passing numeric_only warrants a separate issue. It seems to actually make it more consistent with the other functions in the module. cumprod and cumsum for example, already pass the **kwargs to the self._cython_transform function.

I can add a test if you want but I do believe that it is in the same of this issue I'm addressing so it belongs in this PR. I'm happy to change it (or add a whatsnew + tests) if you want but I don't think its necessarily warranted.

[kprestel]

@jreback I reverted that numeric_only change. I will open up a new issue and PR for that.

Thanks for the help.

[WillAyd]

Thanks @kprestel !