[issue25447] TypeError invoking deepcopy on lru_cache

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Roundup Robot]

Roundup Robot added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Jason R. Coombs]

Jason R. Coombs added the comment:

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 
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.

--
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--
stage: resolved -> patch review
Added file: http://bugs.python.org/file41435/lru_cache_simple_copy.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Jason R. Coombs]

Jason R. Coombs added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Jason R. Coombs]

Jason R. Coombs added the comment:

Wow. That's great. I'll take a look soon... and verify it works with the 
downstream lib.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your report and review Jason.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Roundup Robot]

Roundup Robot added the comment:

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

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

It is redundant. update_wrapper() is called just after calling 
_lru_cache_wrapper() in decorating_function().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Jason R. Coombs]

Jason R. Coombs added the comment:

Simple and elegant. I like it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Jason R. Coombs]

Jason R. Coombs added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Added test cases for nested functions (__qualname__ != __name__).

--
Added file: http://bugs.python.org/file40848/lru_cache_simple_pickling_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--
keywords: +patch
stage:  -> patch review
Added file: http://bugs.python.org/file40845/lru_cache_simple_pickling.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Raymond Hettinger]

Raymond Hettinger added the comment:

> 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.

--
assignee: rhettinger -> serhiy.storchaka
priority: high -> normal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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?

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Jason R. Coombs]

Jason R. Coombs added the comment:

For some context into the error:

$ python3.5 -m pdb cachecopy.py 
> /home/jaraco/cachecopy.py(1)()
-> 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 "", line 1, in 
  File "/home/jaraco/cachecopy.py", line 1, in 
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

(Pdb) args
cls = 
args = ()

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Jason R. Coombs]

New submission from Jason R. Coombs:

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 
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=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.

--
assignee: rhettinger
components: Library (Lib)
keywords: 3.5regression
messages: 253233
nosy: jason.coombs, rhettinger
priority: high
severity: normal
status: open
title: TypeError invoking deepcopy on lru_cache
type: behavior
versions: Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25447] TypeError invoking deepcopy on lru_cache

[Jason R. Coombs]

Jason R. Coombs added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com