Add optional kwargs of BaseQuery to modules and modify tests where necessary - #1419

Hello @kakirastern! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-04-18 16:32:26 UTC

Sorry it took me so long to review this, but now that I'm looking, I don't understand the motivation. Unless a non-default behavior is explicitly required, we do not want to specify all of the optional kwargs. That's generally bad practice, as it adds a lot of redundant text, and it means that any global updates in BaseQuery that are intentionally global (inherited by others) would not propagate.

That's generally bad practice

I would disagree, but we can talk about it later this week. I'm sure we can find a middle ground.

Fair enough. I suppose there's no particular reason that specifying all optional arguments would be bad practice, and indeed it can be necessary for ensuring consistency over time for some operations. But I really don't like it in this case, and if we think we need to specify 10 optional keyword arguments to ensure stability of the call, we have a major design flaw that we should be correcting. I don't think that's the case.

These keyword arguments:

data=None,
headers=None,
files=None,
save=False,
savedir='',
stream=False,
auth=None,

should not be specified, though. Since there is no sensible alternative default for any of these (except, maybe, stream), they would never be globally changed. Specifying them just makes the code extra verbose without reason.

closed at request of author - last SHA 050b590