Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{Core} Python 3.11: Remove uses of deprecated inspect.getargspec #24111

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

mdboom
Copy link

@mdboom mdboom commented Oct 5, 2022

inspect.getargspec has been deprecated since 3.0 in favor of its replacement inspect.getfullargspec.

This does change "vendored" code, and I couldn't find where that may require special treatment, if at all.

There is one other use of inspect.getargspec that is retained for backward compatibility with Python 2 that I didn't touch, but in reality that clause could also probably be removed since Python 2 is no longer supported.

Related command

All az commands.

Description

Another step on the road to Python 3.11 compatibility. (See also #24109)

Testing Guide


This checklist is used to make sure that common guidelines for a pull request are followed.

inspect.getargspec has been deprecated since 3.0 in favor of its replacement inspect.getfullargspec.
@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Oct 5, 2022
@ghost
Copy link

ghost commented Oct 5, 2022

Thank you for your contribution mdboom! We will review the pull request and get back to you soon.

@ghost ghost added Auto-Assign Auto assign by bot Core CLI core infrastructure labels Oct 5, 2022
@ghost ghost requested a review from yonzhan October 5, 2022 20:26
@ghost ghost assigned jiasli Oct 5, 2022
@ghost ghost added this to the Oct 2022 (2022-11-01) milestone Oct 5, 2022
@yonzhan
Copy link
Collaborator

yonzhan commented Oct 5, 2022

remove deprecated dependencies on test

@mdboom
Copy link
Author

mdboom commented Oct 6, 2022

Speaking to @iritkatriel in another forum, it sounds like the most future-proof option is to use inspect.signature. I'm going to update this PR to use that instead.

@mdboom
Copy link
Author

mdboom commented Oct 6, 2022

remove deprecated dependencies on test

Sorry, is this a request of me? I'm not sure I understand.

@mdboom mdboom mentioned this pull request Oct 6, 2022
7 tasks
@wangzelin007
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@@ -59,7 +59,7 @@ def __init__(self, authorization_callback):
# for backwards compatibility we need to support callbacks which don't accept the scheme
def _auth_callback_compat(self, server, resource, scope, scheme):
return self._user_callback(server, resource, scope) \
if len(inspect.getargspec(self._user_callback).args) == 3 \
if len(inspect.signature(self._user_callback).parameters) == 3 \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not equivalent to the original code if self._user_callback contains *args and **kwargs, since signature's result includes *args (kind: VAR_POSITIONAL) and **kwargs (kind: VAR_KEYWORD) in parameters, but getargspec's result has dedicated varargs, keywords elements for them:

import inspect

def testfunc(a=1, b=2, *args, **kwargs):
    pass

print(len(inspect.getfullargspec(testfunc).args))
print(inspect.getfullargspec(testfunc).args)
print(len(inspect.signature(testfunc).parameters))
print(inspect.signature(testfunc).parameters)

Output:

2
['a', 'b']
4
OrderedDict([('a', <Parameter "a=1">), ('b', <Parameter "b=2">), ('args', <Parameter "*args">), ('kwargs', <Parameter "**kwargs">)])

@jiasli jiasli mentioned this pull request Jul 21, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot Core CLI core infrastructure customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants