classification
Title: Document what functions are suitable for use with the lru_cache
Type: Stage: resolved
Components: Documentation Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: bolorsociedad, miss-islington, rhettinger, terry.reedy, vstinner
Priority: normal Keywords: patch

Created on 2018-11-23 11:50 by bolorsociedad, last changed 2018-11-26 10:14 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 10707 merged rhettinger, 2018-11-26 00:19
PR 10708 merged miss-islington, 2018-11-26 00:25
PR 10709 merged miss-islington, 2018-11-26 00:25
Messages (9)
msg330309 - (view) Author: bolorsociedad (bolorsociedad) Date: 2018-11-23 11:53
The decorator functools.lru_cache seems to not work properly when the function to be memoized returns a mutable object.

For instance:

>>> import functools
>>> @functools.lru_cache()
... def f(x):
...    return [x, x + 1]
... 
>>> a = f(4)
>>> print(a)
[4, 5]
>>> a[0] = 0
>>> f(4)
[0, 5]


When the returned mutable object is modified, the cache is modified as well. In my opinion, functools.lru_cache should store a deep copy of the returned object.
msg330313 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-23 12:15
> When the returned mutable object is modified, the cache is modified as well. In my opinion, functools.lru_cache should store a deep copy of the returned object.

It would be inefficient to deep copy the mutable result and can defeat the purpose of the cache...

I would rather to add a note to the documentation to explain to either not return mutable objects or to not modify them :-)
https://docs.python.org/dev/library/functools.html#functools.lru_cache
msg330315 - (view) Author: bolorsociedad (bolorsociedad) Date: 2018-11-23 12:18
I understand it may be inefficient sometimes. Perhaps it would be nice to add an argument to lru_cache to specify that we want to deep copy? Something like

def lru_cache(..., deepcopy=False):
   ...
msg330369 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-11-24 01:16
I concur with Victor.  The proposed API change defeats the purpose of the cache.  By design, the intent of the cache is to reuse the previously computed value.

I can add something like this to the docs: """In general, the LRU cache should only be used when you want to reuse previously computed values.  Accordingly, it doesn't make sense to cache functions with side-effects, functions that need to create distinct mutable objects on each call, or impure functions such as time() or random()."""
msg330381 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-11-25 00:59
The proposed addition looks good to me.
msg330401 - (view) Author: miss-islington (miss-islington) Date: 2018-11-26 00:24
New changeset f0e0f2008d160cdd7e5bc02ea61c43f8c7b2b82f by Miss Islington (bot) (Raymond Hettinger) in branch 'master':
bpo-35300: Add usage note to the lru_cache() docs (GH-10707)
https://github.com/python/cpython/commit/f0e0f2008d160cdd7e5bc02ea61c43f8c7b2b82f
msg330402 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-11-26 01:00
New changeset c7074007272f257a0efab76f03e6b400c30a51b4 by Raymond Hettinger (Miss Islington (bot)) in branch '3.7':
bpo-35300: Add usage note to the lru_cache() docs (GH-10707) (GH-10708)
https://github.com/python/cpython/commit/c7074007272f257a0efab76f03e6b400c30a51b4
msg330403 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-11-26 01:01
New changeset 6c12091ca67e8ec2960652ef7a763d0de772f799 by Raymond Hettinger (Miss Islington (bot)) in branch '3.6':
bpo-35300: Add usage note to the lru_cache() docs (GH-10707) (GH-10709)
https://github.com/python/cpython/commit/6c12091ca67e8ec2960652ef7a763d0de772f799
msg330410 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-26 10:14
Thanks Raymond, I concur that updating the doc is the proper fix :)
History
Date User Action Args
2018-11-26 10:14:45vstinnersetmessages: + msg330410
2018-11-26 01:01:31rhettingersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-11-26 01:01:11rhettingersetmessages: + msg330403
2018-11-26 01:00:44rhettingersetmessages: + msg330402
2018-11-26 00:25:16miss-islingtonsetpull_requests: + pull_request9959
2018-11-26 00:25:06miss-islingtonsetpull_requests: + pull_request9958
2018-11-26 00:24:55miss-islingtonsetnosy: + miss-islington
messages: + msg330401
2018-11-26 00:19:19rhettingersetkeywords: + patch
stage: patch review
pull_requests: + pull_request9957
2018-11-25 00:59:11terry.reedysetnosy: + terry.reedy
messages: + msg330381
2018-11-24 01:19:45rhettingersettitle: Bug with memoization and mutable objects -> Document what functions are suitable for use with the lru_cache
2018-11-24 01:16:45rhettingersetversions: - Python 3.4, Python 3.5
messages: + msg330369

assignee: rhettinger
components: + Documentation, - Library (Lib)
type: behavior ->
2018-11-23 12:18:21bolorsociedadsetmessages: + msg330315
2018-11-23 12:15:29vstinnersetnosy: + rhettinger, vstinner
messages: + msg330313
2018-11-23 11:53:18bolorsociedadsetmessages: + msg330309
2018-11-23 11:50:20bolorsociedadcreate