Repository: joblib/joblib

Issue:
Use inspect.signature() instead of inspect.getfullargspec()
Currently, the implementation of `Memory.cache()` implements the `ignore` option using [`inspect.getfullargspec()`](https://docs.python.org/3/library/inspect.html#inspect.getfullargspec).  However, this function does not handle decorated functions correctly; [`inspect.signature()`](https://docs.python.org/3/library/inspect.html#inspect.signature), on the other hand, does.  Observe:

```python
from functools import wraps
import inspect

def decorate(f):
    @wraps(f)
    def wrapped(*args, **kwargs):
        return f(*args, **kwargs)
    return wrapped

@decorate
def foo(arg, extra=None):
    return arg

print(inspect.signature(foo))
print(inspect.getfullargspec(foo))
```

This prints out:

```
(arg, extra=None)
FullArgSpec(args=[], varargs='args', varkw='kwargs', defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
```

Note that `signature()` reports the signature of the original `foo`, while `getfullargspec()` reports the signature of `wrapped`.

As a consequence of this, using the `ignore` option on a Memory-cached function with an intervening decorator does not work.  Example:

```python
from functools import wraps
import inspect
from joblib import Memory

mem = Memory("cache", verbose=0)

def decorate(f):
    @wraps(f)
    def wrapped(*args, **kwargs):
        return f(*args, **kwargs)
    return wrapped

@mem.cache(ignore=["extra"])
@decorate
def foo(arg, extra=None):
    return arg

foo(1)
```

This fails with the error `ValueError: Ignore list: argument 'extra' is not defined for function foo(*args, **kwargs)`.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/joblib/joblib/pull/1165?src=pr&el=h1) Report
> Merging [#1165](https://codecov.io/gh/joblib/joblib/pull/1165?src=pr&el=desc) (9b8a9f8) into [master](https://codecov.io/gh/joblib/joblib/commit/5c7c4566bc2668a435799713fcbf467234f4e984?el=desc) (5c7c456) will **decrease** coverage by `22.99%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/joblib/joblib/pull/1165/graphs/tree.svg?width=650&height=150&src=pr&token=gA6LF5DGTW)](https://codecov.io/gh/joblib/joblib/pull/1165?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           master    #1165       +/-   ##
===========================================
- Coverage   94.44%   71.44%   -23.00%     
===========================================
  Files          47       47               
  Lines        6964     6818      -146     
===========================================
- Hits         6577     4871     -1706     
- Misses        387     1947     +1560     
```


| [Impacted Files](https://codecov.io/gh/joblib/joblib/pull/1165?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [joblib/func\_inspect.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL2Z1bmNfaW5zcGVjdC5weQ==) | `87.70% <100.00%> (-4.56%)` | :arrow_down: |
| [joblib/test/test\_memory.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL3Rlc3QvdGVzdF9tZW1vcnkucHk=) | `95.85% <100.00%> (-2.71%)` | :arrow_down: |
| [joblib/test/test\_dask.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL3Rlc3QvdGVzdF9kYXNrLnB5) | `3.77% <0.00%> (-95.11%)` | :arrow_down: |
| [joblib/\_dask.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL19kYXNrLnB5) | `23.76% <0.00%> (-70.33%)` | :arrow_down: |
| [joblib/executor.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL2V4ZWN1dG9yLnB5) | `30.95% <0.00%> (-69.05%)` | :arrow_down: |
| [joblib/test/test\_memmapping.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL3Rlc3QvdGVzdF9tZW1tYXBwaW5nLnB5) | `37.07% <0.00%> (-62.17%)` | :arrow_down: |
| [joblib/pool.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL3Bvb2wucHk=) | `27.04% <0.00%> (-60.76%)` | :arrow_down: |
| [joblib/\_memmapping\_reducer.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL19tZW1tYXBwaW5nX3JlZHVjZXIucHk=) | `38.63% <0.00%> (-57.97%)` | :arrow_down: |
| [joblib/\_multiprocessing\_helpers.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL19tdWx0aXByb2Nlc3NpbmdfaGVscGVycy5weQ==) | `28.00% <0.00%> (-52.00%)` | :arrow_down: |
| [joblib/backports.py](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree#diff-am9ibGliL2JhY2twb3J0cy5weQ==) | `39.47% <0.00%> (-44.74%)` | :arrow_down: |
| ... and [27 more](https://codecov.io/gh/joblib/joblib/pull/1165/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/joblib/joblib/pull/1165?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/joblib/joblib/pull/1165?src=pr&el=footer). Last update [5c7c456...9b8a9f8](https://codecov.io/gh/joblib/joblib/pull/1165?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 5328c2c..93144b3 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,6 +1,13 @@
 Latest changes
 ==============
 
+Development version
+-------------------
+
+- Fix joblib.Memory bug with the ``ignore`` parameter when the cached function
+  is a decorated function.
+  https://github.com/joblib/joblib/pull/1165
+
 1.0.1
 -----
 
diff --git a/joblib/func_inspect.py b/joblib/func_inspect.py
index ec6bb4a..3751347 100644
--- a/joblib/func_inspect.py
+++ b/joblib/func_inspect.py
@@ -171,10 +171,9 @@ def get_func_name(func, resolv_alias=True, win_characters=True):
     return module, name
 
 
-def _signature_str(function_name, arg_spec):
+def _signature_str(function_name, arg_sig):
     """Helper function to output a function signature"""
-    arg_spec_str = inspect.formatargspec(*arg_spec)
-    return '{}{}'.format(function_name, arg_spec_str)
+    return '{}{}'.format(function_name, arg_sig)
 
 
 def _function_called_str(function_name, args, kwargs):
@@ -221,20 +220,34 @@ def filter_args(func, ignore_lst, args=(), kwargs=dict()):
             warnings.warn('Cannot inspect object %s, ignore list will '
                           'not work.' % func, stacklevel=2)
         return {'*': args, '**': kwargs}
-    arg_spec = inspect.getfullargspec(func)
-    arg_names = arg_spec.args + arg_spec.kwonlyargs
-    arg_defaults = arg_spec.defaults or ()
-    if arg_spec.kwonlydefaults:
-        arg_defaults = arg_defaults + tuple(arg_spec.kwonlydefaults[k]
-                                            for k in arg_spec.kwonlyargs
-                                            if k in arg_spec.kwonlydefaults)
-    arg_varargs = arg_spec.varargs
-    arg_varkw = arg_spec.varkw
-
+    arg_sig = inspect.signature(func)
+    arg_names = []
+    arg_defaults = []
+    arg_kwonlyargs = []
+    arg_varargs = None
+    arg_varkw = None
+    for param in arg_sig.parameters.values():
+        if param.kind is param.POSITIONAL_OR_KEYWORD:
+            arg_names.append(param.name)
+        elif param.kind is param.KEYWORD_ONLY:
+            arg_names.append(param.name)
+            arg_kwonlyargs.append(param.name)
+        elif param.kind is param.VAR_POSITIONAL:
+            arg_varargs = param.name
+        elif param.kind is param.VAR_KEYWORD:
+            arg_varkw = param.name
+        if param.default is not param.empty:
+            arg_defaults.append(param.default)
     if inspect.ismethod(func):
         # First argument is 'self', it has been removed by Python
         # we need to add it back:
         args = [func.__self__, ] + args
+        # func is an instance method, inspect.signature(func) does not
+        # include self, we need to fetch it from the class method, i.e
+        # func.__func__
+        class_method_sig = inspect.signature(func.__func__)
+        self_name = next(iter(class_method_sig.parameters))
+        arg_names = [self_name] + arg_names
     # XXX: Maybe I need an inspect.isbuiltin to detect C-level methods, such
     # as on ndarrays.
 
@@ -244,7 +257,7 @@ def filter_args(func, ignore_lst, args=(), kwargs=dict()):
     for arg_position, arg_name in enumerate(arg_names):
         if arg_position < len(args):
             # Positional argument or keyword argument given as positional
-            if arg_name not in arg_spec.kwonlyargs:
+            if arg_name not in arg_kwonlyargs:
                 arg_dict[arg_name] = args[arg_position]
             else:
                 raise ValueError(
@@ -252,7 +265,7 @@ def filter_args(func, ignore_lst, args=(), kwargs=dict()):
                     'positional parameter for %s:\n'
                     '     %s was called.'
                     % (arg_name,
-                       _signature_str(name, arg_spec),
+                       _signature_str(name, arg_sig),
                        _function_called_str(name, args, kwargs))
                 )
 
@@ -268,7 +281,7 @@ def filter_args(func, ignore_lst, args=(), kwargs=dict()):
                     raise ValueError(
                         'Wrong number of arguments for %s:\n'
                         '     %s was called.'
-                        % (_signature_str(name, arg_spec),
+                        % (_signature_str(name, arg_sig),
                            _function_called_str(name, args, kwargs))
                     ) from e
 
@@ -296,7 +309,7 @@ def filter_args(func, ignore_lst, args=(), kwargs=dict()):
             raise ValueError("Ignore list: argument '%s' is not defined for "
                              "function %s"
                              % (item,
-                                _signature_str(name, arg_spec))
+                                _signature_str(name, arg_sig))
                              )
     # XXX: Return a sorted list of pairs?
     return arg_dict
diff --git a/joblib/memory.py b/joblib/memory.py
index 424d9fe..f5d59eb 100644
--- a/joblib/memory.py
+++ b/joblib/memory.py
@@ -882,7 +882,10 @@ class Memory(Logger):
             as functions are evaluated.
 
         bytes_limit: int, optional
-            Limit in bytes of the size of the cache.
+            Limit in bytes of the size of the cache. By default, the size of
+            the cache is unlimited.
+            Note: You need to call :meth:`joblib.Memory.reduce_size` to
+            actually reduce the cache size to be less than ``bytes_limit``.
 
         backend_options: dict, optional
             Contains a dictionnary of named parameters used to configure


Test patch:
diff --git a/joblib/test/test_memory.py b/joblib/test/test_memory.py
index ad0ddf4..e917871 100644
--- a/joblib/test/test_memory.py
+++ b/joblib/test/test_memory.py
@@ -6,6 +6,7 @@ Test the memory module.
 # Copyright (c) 2009 Gael Varoquaux
 # License: BSD Style, 3 clauses.
 
+import functools
 import gc
 import shutil
 import os
@@ -488,6 +489,32 @@ def test_memory_ignore(tmpdir):
     assert len(accumulator) == 1
 
 
+def test_memory_ignore_decorated(tmpdir):
+    " Test the ignore feature of memory on a decorated function "
+    memory = Memory(location=tmpdir.strpath, verbose=0)
+    accumulator = list()
+
+    def decorate(f):
+        @functools.wraps(f)
+        def wrapped(*args, **kwargs):
+            return f(*args, **kwargs)
+        return wrapped
+
+    @memory.cache(ignore=['y'])
+    @decorate
+    def z(x, y=1):
+        accumulator.append(1)
+
+    assert z.ignore == ['y']
+
+    z(0, y=1)
+    assert len(accumulator) == 1
+    z(0, y=1)
+    assert len(accumulator) == 1
+    z(0, y=2)
+    assert len(accumulator) == 1
+
+
 def test_memory_args_as_kwargs(tmpdir):
     """Non-regression test against 0.12.0 changes.