Skip to content

Conversation

@Danarvelini
Copy link
Contributor

@Danarvelini Danarvelini commented Nov 29, 2024

Update _client.py

"proxies" param is missing if passed as keyword argument. Even tough it should be there, since it is mentioned in the docstring as an optional param.

Summary

I am adding "proxies" keyword params to optional Client() params. It should already be there to deal with a scenario in which it is going as a keyword argument, even if it is None. I am facing this scenario at the moment and I thought of adding this missing part of the code so others won't need to face it as well.
Also, docstring of the class mentions it, but it is not there in the init params.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

Update _client.py

"proxies" param is missing if passed as keyword argument.
Even tough it should be there, since it is mentioned in the docstring as an optional param.
@lovelydinosaur
Copy link
Contributor

Pinning versions and deprecation processes are tricky aren't they? I don't think including an empty non functioning argument here is helpful.

@Danarvelini
Copy link
Contributor Author

Will this be deprecated?

Shouldn't it be removed from class docstring as an acceptable optional parameter, then?
Right now, it says as acceptable, but actually, it is not supported when passed down to the class.

@lovelydinosaur
Copy link
Contributor

It was deprecated and has now been removed.

You’re correct, it’s still accidentally included in the docstring.
If you’d like to issue a pull request resolving that it’d be welcome.

@Danarvelini
Copy link
Contributor Author

Ok, I just did that. Thank you :)

#3426

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants