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

Use inspect.signature() for Python3 and up #551

Merged
merged 2 commits into from
Nov 30, 2015
Merged

Conversation

13steinj
Copy link
Contributor

inspect.getargspec() is soon to be deprecated in favor of inspect.signature().

inspect.signature was not backported for Python2, so we will keep using inspect.getargspec() for that.

Resolves #541

@13steinj 13steinj force-pushed the fixargspec branch 2 times, most recently from 2b18ed4 to d89a791 Compare November 22, 2015 18:54
@@ -48,7 +48,14 @@ def alias_function(function, class_name):
"""
@wraps(function)
def wrapped(self, *args, **kwargs):
func_args = inspect.getargspec(function).args
if six.PY3 and not hasattr(sys, 'pypy_version_info'):
Copy link
Member

Choose a reason for hiding this comment

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

Can we extract func_args into it's own function to avoid repetition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bboe sure :P would you like it in this file somewhere or in internal.py?

E: or somewhere else, of course.

Copy link
Member

Choose a reason for hiding this comment

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

It could be another nested function within alias_function since it doesn't really fit anywhere else at the moment. If we used it elsewhere, I think internal would make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used in both alias_function and require_captcha, so internal?

Copy link
Member

Choose a reason for hiding this comment

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

Didn't catch that. Yes that would be appropriate.
On Nov 22, 2015 3:38 PM, "13steinj" notifications@github.com wrote:

In praw/decorators.py
#551 (comment):

@@ -48,7 +48,14 @@ def alias_function(function, class_name):
"""
@wraps(function)
def wrapped(self, _args, *_kwargs):

  •    func_args = inspect.getargspec(function).args
    
  •    if six.PY3 and not hasattr(sys, 'pypy_version_info'):
    

It's used in both alias_function and require_captcha, so internal?


Reply to this email directly or view it on GitHub
https://github.com/praw-dev/praw/pull/551/files#r45566954.

@bboe
Copy link
Member

bboe commented Nov 22, 2015

Awesome! Thanks for the pull request. Just one small issue to address then it'll be good to go. The nightly build (3.6 alpha) won't pass until flake8 is updated to support the same change, but that's okay for us.

@13steinj 13steinj force-pushed the fixargspec branch 4 times, most recently from 5f1001e to 569745b Compare November 23, 2015 03:08
@13steinj
Copy link
Contributor Author

@bboe done...But I've no idea why Travis is messing up. And on travis it's showing an imaginary commit.

@13steinj 13steinj force-pushed the fixargspec branch 2 times, most recently from d196120 to a38373f Compare November 23, 2015 04:35
`inspect.getargspec()` is soon to be deprecated in favor of `inspect.signature()`.

`inspect.signature` was not backported for Python2 or PyPy, so we will keep using `inspect.getargspec()` for that.

Resolves praw-dev#541
@13steinj
Copy link
Contributor Author

@bboe I fixed it, but I couldn't add it to internal. I had to add it to decorator_helpers because when getting it from internal there was essentially a weird import loop.

bboe added a commit that referenced this pull request Nov 30, 2015
Use inspect.signature() for Python3 and up
@bboe bboe merged commit 7233edd into praw-dev:master Nov 30, 2015
@bboe
Copy link
Member

bboe commented Nov 30, 2015

Great work!

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