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

Replace deprecated inspect.formatargspec with inspect.signature #2507

Merged
merged 2 commits into from
Jun 16, 2022

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented Sep 30, 2021

inspect.formatargspec is deprecated:

Deprecated since version 3.5: Use signature() and Signature Object, which provide a better introspecting API for callables.

https://docs.python.org/3/library/inspect.html#inspect.formatargspec

python/cpython#28618

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2021

Codecov Report

Merging #2507 (1d47590) into develop (098cc25) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #2507      +/-   ##
===========================================
- Coverage    95.30%   95.30%   -0.01%     
===========================================
  Files           60       60              
  Lines        12262    12261       -1     
===========================================
- Hits         11686    11685       -1     
  Misses         576      576              
Impacted Files Coverage Δ
botocore/docs/method.py 100.00% <100.00%> (ø)

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 098cc25...1d47590. Read the comment docs.

@nateprewitt
Copy link
Contributor

Thanks for the PR @hugovk! We'd started some of this work but weren't sure what the impact of this change is on documentation generation. I think the only outstanding check is validating we're not adding additional signature footprint (specifically with the inclusion of self) in this change.

I believe the move to Signature, and no longer using [1:] for args, may be an issue.

@mgorny
Copy link
Contributor

mgorny commented May 14, 2022

Ping. This is now breaking botocore on Python 3.11, where inspect.formatargspec() was removed.

mgorny added a commit to mgorny/botocore that referenced this pull request May 14, 2022
Originally submitted by Hugo van Kemenade as boto#2507.  Modified by me
to remove the first positional parameter like the old code did.
@mgorny
Copy link
Contributor

mgorny commented May 14, 2022

I've submitted #2673 as a little more elaborate approach that preserves the current behavior of skipping "self" (or more precisely, the first positional parameter). I hope you don't mind me modifying your code.

@hugovk
Copy link
Contributor Author

hugovk commented May 14, 2022

Absolutely not, go for it!

@nateprewitt
Copy link
Contributor

Thanks @mgorny, we’re waiting for the last reformatting PR to get merged in the next week or two. Then we plan to drop Python 3.6 on May 30th, at which point we’ll merge this and start testing on the 3.11dev build.

@mgorny
Copy link
Contributor

mgorny commented May 14, 2022

Thanks. It seems that I need to work on it some more but I should finish it today.

mgorny added a commit to mgorny/botocore that referenced this pull request May 14, 2022
Originally submitted by Hugo van Kemenade as boto#2507.  Modified by me
to remove the first positional parameter like the old code did.
@dlm6693 dlm6693 self-requested a review June 16, 2022 16:57
@dlm6693
Copy link
Contributor

dlm6693 commented Jun 16, 2022

@hugovk thank you for submitting this original PR and thank you @mgorny for following up with the impending release of 3.11! However, we will be merging this original one as inspect.signature implicitly does NOT include self as an argument, which is the desired behavior for botocore doc generation. I tested this specific function as well as generated diffs for many of our service docs both before and after implementing this change and found that there weren't any

@dlm6693
Copy link
Contributor

dlm6693 commented Jun 16, 2022

@mgorny these are a few manual tests that I ran:

import inspect
from botocore.loaders import Loader

class Foo:
    def test(self, a, b):
        pass

f = Foo()
inspect.signature(f.test)
<Signature (a, b)>

l = Loader()
inspect.signature(l._potential_locations)
<Signature (name=None, must_exist=False, is_dir=False)>

Note that self is not included in either Signature object

@dlm6693 dlm6693 merged commit 2396625 into boto:develop Jun 16, 2022
@hugovk hugovk deleted the replace-deprecated branch June 16, 2022 17:51
nateprewitt pushed a commit to nateprewitt/botocore that referenced this pull request Jun 16, 2022
Originally submitted by Hugo van Kemenade as boto#2507.  Modified by me
to remove the first positional parameter like the old code did.
nateprewitt added a commit that referenced this pull request Jun 16, 2022
…2698)

Originally submitted by Hugo van Kemenade as #2507.  Modified by me
to remove the first positional parameter like the old code did.

Co-authored-by: Michał Górny <mgorny@gentoo.org>
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.

None yet

5 participants