bpo-33911: Fixed deprecation warning in XML-RPC ServerHTMLDoc. #7847
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. When your account is ready, please add a comment in this pull request Thanks again for your contribution, we look forward to reviewing it! |
|
Does your PR change the output? If yes, it should be documented with a new NEWS entry (use the "blurb" tool to add it: https://devguide.python.org/committing/#what-s-new-and-news-entries ). |
|
@1st1: Would you mind to have a look? |
Lib/xmlrpc/server.py
Outdated
| annotations=args.annotations, | ||
| formatvalue=self.formatvalue | ||
| ) | ||
| sig.replace(parameters=tuple(sig.parameters.values())[1:]) |
There was a problem hiding this comment.
sig.replace() returns a new Signature object, so you should have written sig = sig.replace(...). But inspect.signature should take care of the bound argument automatically, so you don't need this dance. if and elif clauses should be replaced with
if callable(object):
argspec = str(signature(object))
else:
argspec = '(...)'
There was a problem hiding this comment.
How common a failure of signature()? Shouldn't we catch some common exceptions?
There was a problem hiding this comment.
inspect.getfullargspec uses inspect.signature behind the scenes. So if you want to add some error handling, it should be in a separate PR.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I have made the requested changes; please review again. @vstinner: AFAIK, the output doesn't change (HTML output is compared in various test methods in |
|
Thanks for making the requested changes! @1st1: please review the changes made to this pull request. |
vstinner
dismissed
1st1’s stale review
The PR no longer uses sig.replace() but just: argspec = str(signature(object))
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
"./python -W error -m test test_docxmlrpc" now pass, so the PR fixed the issue that I reported.
test_docxmlrpc still pass which means that the HTML is not modified by this change.
|
@1st1: Would you mind to review again the change? |
|
GH-8294 is a backport of this pull request to the 3.7 branch. |
Replace deprecated inspect.getfullargspec() with inspect.signature(). (cherry picked from commit 35c0809) Co-authored-by: Nicolas Noé <[email protected]>
) Replace deprecated inspect.getfullargspec() with inspect.signature(). (cherry picked from commit 35c0809) Co-authored-by: Nicolas Noé <[email protected]>
7 participants
https://bugs.python.org/issue33911