Repository: scrapy/scrapy
Pull request: Use getfullargspec under the scenes for py3 to stop DeprecationWarning
fixes #2862

I also ran `tox` for the tests, but there are slight differences for the packages used inside the library, so in `tests/requirements.txt`:
- `netlib` can be easily updated to `0.17` to support both py2 and py3
- `mitmproxy` on the other hand can't w/o some modifications; its version `0.18+` that supports both py3 and py2 doesn't contain `libmproxy` that's used in `tests/test_proxy_connect.py`. So yea, related to https://github.com/scrapy/scrapy/issues/2545 actually, so nevermind.

Other than that issue, everything worked fine, no errors for py2 nor py3.

Also I tested the branch in our project and the `DeprecationWarning` has disappeared now :).

Problem statement:
DeprecationWarning: inspect.getargspec()
Hi,

I've tried searching for a similar problem but couldn't find a discussion in scrapy about it, so starting one.

```bash
build  │   func_args, _, _, _ = inspect.getargspec(func)
build  │ /usr/local/lib/python3.6/site-packages/scrapy/utils/python.py:201: DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() or inspect.getfullargspec()
```

We've stumbled into this log a LOT, actually getting spammed by it currently when we execute some `twisted.trial.unittest` tests where each unittest spins up a `CrawlerRunner` for our live tests that do exactly what unittests do but with live data fetching.

Could we leave the current behavior for py2 but port the `getargspec` to `getfullargspec` as is suggested in the warning? I see latter for py3 includes additional return values, but should be a quick port. Would you be ok with a patch? Thanks

Hints from issue discussion:
Sure, it would be a good addition!

Commit message: Add utility function instead of patching inspect.getargspec. #2864

Patch to review:
diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py
index 72f8f43118d..ab75be78bd4 100644
--- a/scrapy/utils/python.py
+++ b/scrapy/utils/python.py
@@ -10,7 +10,6 @@
 import six
 from functools import partial, wraps
 import sys
-import time
 
 from scrapy.utils.decorators import deprecated
 
@@ -198,10 +197,30 @@ def binary_is_text(data):
     return all(c not in _BINARYCHARS for c in data)
 
 
+def getargspec_py23(func):
+    """getargspec_py23(function) -> named tuple ArgSpec(args, varargs, keywords,
+                                                        defaults)
+
+    Identical to inspect.getargspec() in python2, but uses
+    inspect.getfullargspec() for python3 behind the scenes to avoid
+    DeprecationWarning.
+
+    >>> def f(a, b=2, *ar, **kw):
+    ...     pass
+
+    >>> getargspec_py23(f)
+    ArgSpec(args=['a', 'b'], varargs='ar', keywords='kw', defaults=(2,))
+    """
+    if six.PY3:
+        full_argspec = list(inspect.getfullargspec(func))
+        return inspect.ArgSpec(*full_argspec[:4])
+    return inspect.getargspec(func)
+
+
 def get_func_args(func, stripself=False):
     """Return the argument name list of a callable"""
     if inspect.isfunction(func):
-        func_args, _, _, _ = inspect.getargspec(func)
+        func_args, _, _, _ = getargspec_py23(func)
     elif inspect.isclass(func):
         return get_func_args(func.__init__, True)
     elif inspect.ismethod(func):
@@ -248,9 +267,9 @@ def get_spec(func):
     """
 
     if inspect.isfunction(func) or inspect.ismethod(func):
-        spec = inspect.getargspec(func)
+        spec = getargspec_py23(func)
     elif hasattr(func, '__call__'):
-        spec = inspect.getargspec(func.__call__)
+        spec = getargspec_py23(func.__call__)
     else:
         raise TypeError('%s is not callable' % type(func))
 


Code review comments:
diff_hunk: @@ -198,10 +197,30 @@ def binary_is_text(data):
     return all(c not in _BINARYCHARS for c in data)
 
 
+def getargspec_py23(func):
+    """getargspec_py23(function) -> named tuple ArgSpec(args, varargs, keywords,
+                                                        defaults)
+
+    Identical to inspect.getargspec() in python2, but uses
+    inspect.getfullargspec() for python3 behind the scenes to avoid
+    DeprecationWarning.
+
+    >>> def f(a, b=2, *ar, **kw):
+    ...     pass
+
+    >>> getargspec_py23(f)
+    ArgSpec(args=['a', 'b'], varargs='ar', keywords='kw', defaults=(2,))
+    """
+    if six.PY3:
path: scrapy/utils/python.py
text: @user2:
If I recall, @user1 prefers `if not six.PY2`

@user3:
I would go the other way around:

```python
if six.PY2:
    return inspect.getargspec(func)

return inspect.ArgSpec(*inspect.getfullargspec(func)[:4])
```

@user3:
And I would make this function private, so we reserve the right to drop it if not needed by other utils in this module. 

@author:
ok, makes sense to do that as long as usage of `inspect.getargspec` stays within the `utils.python` module, as is now. If at some point `inspect.getargspec` gets used outside, it probably makes sense to do `from scrapy.utils.python import getargspec` instead of `from inspect import getargspec` to be able to use that for both py2 and py3 without triggering deprecation warning.