bpo-38565: add new cache_parameters method for lru_cache #16916
Conversation
Lib/functools.py
Outdated
| @@ -613,6 +613,9 @@ def cache_info(): | |||
| with lock: | |||
| return _CacheInfo(hits, misses, maxsize, cache_len()) | |||
|
|
|||
| def cache_parameters(): | |||
| return {"maxsize": maxsize, "typed": typed} | |||
There was a problem hiding this comment.
NIt: Please use single quotes, the same as used by dict.__repr__()
There was a problem hiding this comment.
Also move the function to just after cache_clear().
Lib/test/test_functools.py
Outdated
| def test_lru_cache_parameters(self): | ||
| def f(): | ||
| return 1 | ||
| f = self.module.lru_cache(2)(f) |
There was a problem hiding this comment.
Let's use the decorator notation to create the example. Also, make the example use maxsize=1000 and typed=True.
| @@ -0,0 +1 @@ | |||
| add new cache_parameters method for lru_cache | |||
There was a problem hiding this comment.
``Add new cache_parameters() method for functools.lru_cache() to better support pickling.
Modules/_functoolsmodule.c
Outdated
| static PyObject * | ||
| lru_cache_cache_parameters(lru_cache_object *self) | ||
| { | ||
| PyObject *cache_parameters = PyDict_New(); |
There was a problem hiding this comment.
Need to test whether cache_parameters is NULL.
Modules/_functoolsmodule.c
Outdated
| lru_cache_cache_parameters(lru_cache_object *self) | ||
| { | ||
| PyObject *cache_parameters = PyDict_New(); | ||
| PyDict_SetItemString(cache_parameters, "maxsize", PyLong_FromSsize_t(self->maxsize)); |
There was a problem hiding this comment.
There are also two possible failure points that need to be tested here as well.
Modules/_functoolsmodule.c
Outdated
| { | ||
| PyObject *cache_parameters = PyDict_New(); | ||
| PyDict_SetItemString(cache_parameters, "maxsize", PyLong_FromSsize_t(self->maxsize)); | ||
| PyDict_SetItemString(cache_parameters, "typed", self->typed == 0 ? Py_False : Py_True); |
There was a problem hiding this comment.
- Drop the test for zero, putting the positive case first:
self->typed ? Py_True : Py_False) - Test the result of PyDict_SetItemString() to see if it failed.
Modules/_functoolsmodule.c
Outdated
| @@ -1335,6 +1344,7 @@ cache_info_type: namedtuple class with the fields:\n\ | |||
|
|
|||
| static PyMethodDef lru_cache_methods[] = { | |||
| {"cache_info", (PyCFunction)lru_cache_cache_info, METH_NOARGS}, | |||
| {"cache_parameters", (PyCFunction)lru_cache_cache_parameters, METH_NOARGS}, | |||
There was a problem hiding this comment.
Move this down one, just after the cache_clear entry.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Zheaoli
force-pushed
the
bpo-38565
branch
2 times, most recently
from
8354f2d to
37f4f0d
Compare
… support pickling
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @rhettinger: please review the changes made to this pull request. |
|
ping @rhettinger |
https://bugs.python.org/issue38565
https://bugs.python.org/issue38565