Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose the value passed of typed passed to functools.lru_cache #82746

Closed
ScottSanderson2 mannequin opened this issue Oct 23, 2019 · 13 comments
Closed

Expose the value passed of typed passed to functools.lru_cache #82746

ScottSanderson2 mannequin opened this issue Oct 23, 2019 · 13 comments
Assignees
Labels
3.9 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ScottSanderson2
Copy link
Mannequin

ScottSanderson2 mannequin commented Oct 23, 2019

BPO 38565
Nosy @rhettinger, @serhiy-storchaka, @Zheaoli
PRs
  • bpo-38565: expose typed filed for lru_cache().cache_info() #16915
  • bpo-38565: add new cache_parameters method for lru_cache #16916
  • 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/rhettinger'
    closed_at = <Date 2019-11-12.07:32:24.952>
    created_at = <Date 2019-10-23.14:50:54.379>
    labels = ['type-feature', 'library', '3.9']
    title = 'Expose the value passed of typed passed to functools.lru_cache'
    updated_at = <Date 2019-11-12.07:34:21.149>
    user = 'https://bugs.python.org/ScottSanderson2'

    bugs.python.org fields:

    activity = <Date 2019-11-12.07:34:21.149>
    actor = 'Manjusaka'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2019-11-12.07:32:24.952>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2019-10-23.14:50:54.379>
    creator = 'Scott Sanderson2'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38565
    keywords = ['patch']
    message_count = 13.0
    messages = ['355226', '355277', '355284', '355285', '355286', '355288', '355291', '355296', '355297', '355304', '356418', '356419', '356420']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'serhiy.storchaka', 'Scott Sanderson2', 'Manjusaka']
    pr_nums = ['16915', '16916']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue38565'
    versions = ['Python 3.9']

    @ScottSanderson2
    Copy link
    Mannequin Author

    ScottSanderson2 mannequin commented Oct 23, 2019

    In some circumstances, it's useful to be able in inspect the parameters with which an instance of functools.lru_cache was instantiated. It's currently possible to recover the cache's maxsize via the .cache_info() method, but there's no way to recover the value passed for typed, which controls whether the lru_cache's cache is partitioned by input type.

    This came up in the context of cloudpickle, a library that tries to extend pickle to support more types (in particular, interactively-defined functions and classes) for use-cases like cluster computing.

    It's currently not possible to pickle an lru-cache decorated function that's defined in main (which includes, e.g. a Jupyter Notebook). We can almost fix this with a pure library solution (see cloudpipe/cloudpickle#309), but we're currently blocked by the fact that there's no way to recover the value that was passed for typed. Exposing a .typed attribute on the extension type for lru_cached functions fixes this

    For more discussion, see the above linked PR, along with cloudpipe/cloudpickle#178.

    @ScottSanderson2 ScottSanderson2 mannequin added 3.9 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Oct 23, 2019
    @Zheaoli
    Copy link
    Mannequin

    Zheaoli mannequin commented Oct 24, 2019

    I think it's a good idea to expose the full arguments that people use in lru_cache()

    @Zheaoli
    Copy link
    Mannequin

    Zheaoli mannequin commented Oct 24, 2019

    I have already make a PR for this issue

    but here's a problem. add a new field to cache_info maybe cause some special problem for the third-party libs what're dependent the cache_info

    @rhettinger
    Copy link
    Contributor

    I'm fine with adding a new function:

       >>> f.cache_parameters()
       {'maxsize': 200, 'typed'=False}

    @rhettinger rhettinger self-assigned this Oct 24, 2019
    @Zheaoli
    Copy link
    Mannequin

    Zheaoli mannequin commented Oct 24, 2019

    I think add new function would be a better way

    I will make a new PR

    @serhiy-storchaka
    Copy link
    Member

    It is easy:

    diff --git a/Lib/functools.py b/Lib/functools.py
    index 3192bd02d9..52c07db749 100644
    --- a/Lib/functools.py
    +++ b/Lib/functools.py
    @@ -499,7 +499,9 @@ def lru_cache(maxsize=128, typed=False):
             # The user_function was passed in directly via the maxsize argument
             user_function, maxsize = maxsize, 128
             wrapper = _lru_cache_wrapper(user_function, maxsize, typed, _CacheInfo)
    -        return update_wrapper(wrapper, user_function)
    +        func = update_wrapper(wrapper, user_function)
    +        func.cache_parameters = lambda: {'maxsize': maxsize, 'typed': typed}
    +        return func
         elif maxsize is not None:
             raise TypeError(
                 'Expected first argument to be an integer, a callable, or None')

    But there are many design questions. Why method instead of just attribute?

            func.cache_parameters = {'maxsize': maxsize, 'typed': typed}

    Or maybe just add the "typed" attribute?

            func.typed = typed

    Also I consider adding the more general "make_key" parameter to lru_cache(). The "typed" parameter would just specify the default value for "make_key".

    @Zheaoli
    Copy link
    Mannequin

    Zheaoli mannequin commented Oct 24, 2019

    I think to modify in lru_cache should be good in normal circumstance

    But here's maybe some people modify the wrapped object that underlying in lru_cache

    So I prefer to add a new function in _functools.c?

    @Zheaoli
    Copy link
    Mannequin

    Zheaoli mannequin commented Oct 24, 2019

    I have already make a new function like this

    static PyObject *
    lru_cache_cache_parameters(lru_cache_object *self)
    {
        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);
        return cache_parameters;
    }

    @serhiy-storchaka
    Copy link
    Member

    The rule used in the lru_cache implementation is: do not write in C that can be written in Python.

    @Zheaoli
    Copy link
    Mannequin

    Zheaoli mannequin commented Oct 24, 2019

    Yes, you are right, we shouldn't consider about the unstandard using way. I will update my PR

    BTW, what're means for the "make_key" parameter?

    @rhettinger
    Copy link
    Contributor

    New changeset 051ff52 by Raymond Hettinger (Manjusaka) in branch 'master':
    bpo-38565: add new cache_parameters method for lru_cache (GH-16916)
    051ff52

    @rhettinger
    Copy link
    Contributor

    Scott, thank you for the suggestion.
    Serhiy, thank for pointing to the simplest implementation.
    Zheaoli, thanks for the patch.

    @Zheaoli
    Copy link
    Mannequin

    Zheaoli mannequin commented Nov 12, 2019

    Raymond, thanks for fixing many errors for my patch!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants