Repository navigation
TypeError invoking deepcopy on lru_cache #69633
Description
| BPO | 25447 |
|---|---|
| Nosy | @rhettinger, @jaraco, @serhiy-storchaka |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-12-28.22:03:29.301>
created_at = <Date 2015-10-20.16:32:09.067>
labels = ['type-bug', 'library']
title = 'TypeError invoking deepcopy on lru_cache'
updated_at = <Date 2015-12-28.22:03:29.300>
user = 'https://github.com/jaraco'bugs.python.org fields:
activity = <Date 2015-12-28.22:03:29.300>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-12-28.22:03:29.301>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2015-10-20.16:32:09.067>
creator = 'jaraco'
dependencies = []
files = ['40845', '40848', '41435']
hgrepos = []
issue_num = 25447
keywords = ['patch', '3.5regression']
message_count = 17.0
messages = ['253233', '253234', '253239', '253255', '253366', '253367', '253373', '253388', '253389', '253390', '253397', '253398', '257084', '257086', '257103', '257104', '257132']
nosy_count = 4.0
nosy_names = ['rhettinger', 'jaraco', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25447'
versions = ['Python 3.5', 'Python 3.6']
Activity
jaraco commentedon Oct 20, 2015
Beginning with Python 3.5, an lru_cache no longer survives a deepcopy.
I suspect this is due to the C implementation per bpo-14373.
I realize one's first reaction might be "why are you deep copying a function," so here's the downstream bug and offended code. 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.
jaraco commentedon Oct 20, 2015
For some context into the error:
serhiy-storchaka commentedon Oct 20, 2015
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?
jaraco commentedon Oct 20, 2015
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.
rhettinger commentedon Oct 23, 2015
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.
serhiy-storchaka commentedon Oct 23, 2015
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.
serhiy-storchaka commentedon Oct 23, 2015
Added test cases for nested functions (qualname != __name__).
jaraco commentedon Oct 23, 2015
Simple and elegant. I like it.
jaraco commentedon Oct 23, 2015
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?
serhiy-storchaka commentedon Oct 23, 2015
It is redundant. update_wrapper() is called just after calling _lru_cache_wrapper() in decorating_function().
python-dev commentedon Oct 24, 2015
New changeset 73226f0d0f89 by Serhiy Storchaka in branch '3.5':
Issue bpo-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 bpo-25447: The lru_cache() wrapper objects now can be copied and pickled
https://hg.python.org/cpython/rev/d3e048c7b65a
serhiy-storchaka commentedon Oct 24, 2015
Thank you for your report and review Jason.
jaraco commentedon Dec 27, 2015
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 and the offended code.
I've since realized, thanks to the test suite on jaraco.functools, a follow-on issue 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, partialI 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.
serhiy-storchaka commentedon Dec 27, 2015
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.
jaraco commentedon Dec 28, 2015
Wow. That's great. I'll take a look soon... and verify it works with the downstream lib.
jaraco commentedon Dec 28, 2015
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-dev commentedon Dec 28, 2015
New changeset 33b428ef34b9 by Serhiy Storchaka in branch '3.5':
Issue bpo-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 bpo-25447: Copying the lru_cache() wrapper object now always works,
https://hg.python.org/cpython/rev/a0f9a8bee85f