classification
Title: TypeError invoking deepcopy on lru_cache
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: jaraco, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: 3.5regression, patch

Created on 2015-10-20 16:32 by jaraco, last changed 2015-12-28 22:03 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
lru_cache_simple_pickling.patch serhiy.storchaka, 2015-10-23 08:05 review
lru_cache_simple_pickling_2.patch serhiy.storchaka, 2015-10-23 09:50 review
lru_cache_simple_copy.patch serhiy.storchaka, 2015-12-27 18:19 review
Messages (17)
msg253233 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2015-10-20 16:32
Beginning with Python 3.5, an lru_cache no longer survives a deepcopy.

```
$ cat >> cache_copy.py
import copy
from functools import lru_cache
foo = lru_cache()(lambda: None)
copy.deepcopy(foo)
$ python3.5 cache_copy.py
Traceback (most recent call last):
  File "cache_copy.py", line 4, in <module>
    copy.deepcopy(foo)
  File "/usr/lib/python3.5/copy.py", line 182, in deepcopy
    y = _reconstruct(x, rv, 1, memo)
  File "/usr/lib/python3.5/copy.py", line 293, in _reconstruct
    y = callable(*args)
  File "/usr/lib/python3.5/copyreg.py", line 88, in __newobj__
    return cls.__new__(cls, *args)
TypeError: Required argument 'user_function' (pos 1) not found
```

I suspect this is due to the C implementation per issue14373.

I realize one's first reaction might be "why are you deep copying a function," so here's the [downstream bug](https://bitbucket.org/jaraco/jaraco.functools/issues/1/method_cache-causes-typeerror-when) and [offended code](https://bitbucket.org/jaraco/jaraco.functools/src/68d1fda21af7e7b4c36577f953f382270bdc1e05/jaraco/functools.py?at=default&fileviewer=file-view-default#functools.py-72:138). That decorator is designed to wrap a method and to store the cache on the instance object, an object which is liable to be deep copied. As a result, production code using this technique is failing on Python 3.5.

Although it was apparently an artifact of previous implementations that the cache happened to be deep-copyable, I hope this expectation can be restored with minimal fuss.
msg253234 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2015-10-20 16:35
For some context into the error:

$ python3.5 -m pdb cachecopy.py 
> /home/jaraco/cachecopy.py(1)<module>()
-> import copy
(Pdb) c
Traceback (most recent call last):
  File "/usr/lib/python3.5/pdb.py", line 1661, in main
    pdb._runscript(mainpyfile)
  File "/usr/lib/python3.5/pdb.py", line 1542, in _runscript
    self.run(statement)
  File "/usr/lib/python3.5/bdb.py", line 431, in run
    exec(cmd, globals, locals)
  File "<string>", line 1, in <module>
  File "/home/jaraco/cachecopy.py", line 1, in <module>
    import copy
  File "/usr/lib/python3.5/copy.py", line 182, in deepcopy
    y = _reconstruct(x, rv, 1, memo)
  File "/usr/lib/python3.5/copy.py", line 293, in _reconstruct
    y = callable(*args)
  File "/usr/lib/python3.5/copyreg.py", line 88, in __newobj__
    return cls.__new__(cls, *args)
TypeError: Required argument 'user_function' (pos 1) not found
Uncaught exception. Entering post mortem debugging
Running 'cont' or 'step' will restart the program
> /usr/lib/python3.5/copyreg.py(88)__newobj__()
-> return cls.__new__(cls, *args)
(Pdb) cls
<class 'functools._lru_cache_wrapper'>
(Pdb) args
cls = <class 'functools._lru_cache_wrapper'>
args = ()
msg253239 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-20 18:08
I'm not sure that we can say that deepcopying was not supported for lru_cache. copy.deepcopy() returned the same object, as for immutable objects, but can we consider decorated with lru_cache() function immutable?
 
The question is: if made functools._lru_cache_wrapper to support deepcopy protocol, should the result of copy.deepcopy() share the cache with original?
msg253255 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2015-10-20 21:14
Indeed. I guess my point about "supported" related to not having tests capturing this requirement or docs stipulating it.

My instinct on the latter question is this - an lru_cache-decorated function is a stateful function. It is mutable, much like a dict or list. If simply copied, the resulting function should have references to the same state. If _deep_ copied, the state (cache) should be similarly deep copied.

Focusing on the deep copy operation, if a cached function is copied, the copy could have additional operations invoked on it and its cache would hit or miss on those calls independently from the original function, and likewise subsequent calls on the original function should not hit on calls unique to the copied function.

I don't have a heavy investment in this expectation. It's also reasonable to me that a deepcopy operation could reuse the same cache or result in an uninitialized one.
msg253366 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-10-23 05:18
> Although it was apparently an artifact of previous 
> implementations that the cache happened to be deep-copyable, 
> I hope this expectation can be restored with minimal fuss.

Hmm, I'm surprised that a deepcopy ever worked.  The LRU cache certainly wasn't designed for that.

If Serhiy has a straight-forward way of restoring the behavior (and adding a test for it), that would be reasonable for 3.5.1.  However, if it involves twisting the code into knots or risking breakage, it would be better to either close this as "won't fix" or reclassify as a feature request for 3.6.
msg253367 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-23 08:05
Using lru_cache() is an implementation detail. I think that lru_cache wrapper should be perfect wrapper, it should support all protocols that are supported by wrapped function (at the same extent as original Python implementation).

Proposed simple patch adds support for pickling, copying and deep-copying lru_cache wrappers. lru_cache wrapper is serialized as global object, i.e. deepcopy() returns the same object, as was in 3.4. Implementing true deep copying (with any semantic) is not so easy, especially for Python implementation.
msg253373 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-23 09:50
Added test cases for nested functions (__qualname__ != __name__).
msg253388 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2015-10-23 16:29
Simple and elegant. I like it.
msg253389 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2015-10-23 16:34
Can you speak to why 'update_wrapper' should be removed? Is that indicative of a deeper issue with update_wrapper also not supporting pickle/deepcopy?
msg253390 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-23 17:01
It is redundant. update_wrapper() is called just after calling _lru_cache_wrapper() in decorating_function().
msg253397 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-24 06:52
New changeset 73226f0d0f89 by Serhiy Storchaka in branch '3.5':
Issue #25447: The lru_cache() wrapper objects now can be copied and pickled
https://hg.python.org/cpython/rev/73226f0d0f89

New changeset d3e048c7b65a by Serhiy Storchaka in branch 'default':
Issue #25447: The lru_cache() wrapper objects now can be copied and pickled
https://hg.python.org/cpython/rev/d3e048c7b65a
msg253398 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-24 06:55
Thank you for your report and review Jason.
msg257084 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2015-12-27 17:09
I've since moved the jaraco.functools project to Github, so here are updated links for those found in the original post to the [downstream bug](https://github.com/jaraco/jaraco.functools/issues/1) and the [offended code](https://github.com/jaraco/jaraco.functools/blob/56d1daf4b88239a137f03da87d312f2522df1078/jaraco/functools.py#L79-L145).

I've since realized, thanks to the test suite on jaraco.functools, a [follow-on issue](https://github.com/jaraco/jaraco.functools/issues/3) where it seems that if the deepcopy occurs on a wrapped partial, it will still fail, because the lru cache wrapper has no qualname:

$ cat > cache_copy.py
import copy
from functools import lru_cache, partial

foo = lru_cache()(partial(print, 'out'))
copy.deepcopy(foo)

$ python3.5 cache_copy.py
Traceback (most recent call last):
  File "cache_copy.py", line 5, in <module>
    copy.deepcopy(foo)
  File "/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/copy.py", line 174, in deepcopy
    rv = reductor(4)
AttributeError: 'functools._lru_cache_wrapper' object has no attribute '__qualname__'

I realize now that when I reviewed the patch, I did not take the time to test it against my full use case, but only relied on the tests as committed, which did not cover this more nuanced usage.

On further consideration, this error highlights a potential subtle difference in the reduce support between the C implementation and the Python implementation.

In digging into the implementation, I don't yet fully understand how it is that the Python implementation survives a deepcopy, but I'd like to revisit this issue to see if it may be possible to make it so.
msg257086 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-27 18:19
The Python implementation survives a deepcopy because the Python implementation has a function type, and functions functions are copied as atoms.

Proposed patch just implements __copy__ and __deepcopy__ methods that returns the object itself for the lru_cache object.
msg257103 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2015-12-28 00:16
Wow. That's great. I'll take a look soon... and verify it works with the downstream lib.
msg257104 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2015-12-28 01:37
I've confirmed this change corrects the failing test and expectation in the downstream project. The tests look good to me as far as covering this more nuanced expectation. Well done, and huge thanks.
msg257132 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-12-28 22:01
New changeset 33b428ef34b9 by Serhiy Storchaka in branch '3.5':
Issue #25447: Copying the lru_cache() wrapper object now always works,
https://hg.python.org/cpython/rev/33b428ef34b9

New changeset a0f9a8bee85f by Serhiy Storchaka in branch 'default':
Issue #25447: Copying the lru_cache() wrapper object now always works,
https://hg.python.org/cpython/rev/a0f9a8bee85f
History
Date User Action Args
2015-12-28 22:03:29serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2015-12-28 22:01:55python-devsetmessages: + msg257132
2015-12-28 01:37:15jaracosetmessages: + msg257104
2015-12-28 00:16:43jaracosetmessages: + msg257103
2015-12-27 18:19:04serhiy.storchakasetfiles: + lru_cache_simple_copy.patch

messages: + msg257086
stage: resolved -> patch review
2015-12-27 17:09:39jaracosetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg257084
2015-10-24 06:55:08serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg253398

stage: patch review -> resolved
2015-10-24 06:52:53python-devsetnosy: + python-dev
messages: + msg253397
2015-10-23 17:01:43serhiy.storchakasetmessages: + msg253390
2015-10-23 16:34:25jaracosetmessages: + msg253389
2015-10-23 16:29:15jaracosetmessages: + msg253388
2015-10-23 09:50:59serhiy.storchakasetfiles: + lru_cache_simple_pickling_2.patch

messages: + msg253373
2015-10-23 08:05:15serhiy.storchakasetfiles: + lru_cache_simple_pickling.patch
keywords: + patch
messages: + msg253367

stage: patch review
2015-10-23 05:18:02rhettingersetpriority: high -> normal
assignee: rhettinger -> serhiy.storchaka
messages: + msg253366
2015-10-20 21:14:41jaracosetmessages: + msg253255
2015-10-20 18:08:26serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg253239
2015-10-20 16:35:15jaracosetmessages: + msg253234
2015-10-20 16:32:09jaracocreate