support python3 keyword-only arguments for function decorator

Issue #96 new
Anonymous created an issue

getargspec() is deprecated in python 3.0

When running against 3.5, I get the following ValueError...

File "~/.pyenv/versions/env/lib/python3.5/site-packages/dogpile/cache/region.py", line 1037, in decorator key_generator = function_key_generator(namespace, fn) File "~/.pyenv/versions/env/lib/python3.5/site-packages/dogpile/cache/util.py", line 73, in function_key_generator args = inspect.getargspec(fn) File "~/.pyenv/versions/3.5.0/lib/python3.5/inspect.py", line 1044, in getargspec raise ValueError("Function has keyword-only arguments or annotations" ValueError: Function has keyword-only arguments or annotations, use getfullargspec() API which can support them

Comments (15)

  1. Mark Heppner

    I'm having the same issue, Python 3.6. Here's a formatted stack trace:

    ~/Documents/app/env/lib64/python3.6/site-packages/dogpile/cache/region.py in decorator(fn)
       1213             if to_str is compat.string_type:
       1214                 # backwards compatible
    -> 1215                 key_generator = function_key_generator(namespace, fn)
       1216             else:
       1217                 key_generator = function_key_generator(
    
    ~/Documents/app/env/lib64/python3.6/site-packages/dogpile/cache/util.py in function_key_generator(namespace, fn, to_str)
         29         namespace = '%s:%s|%s' % (fn.__module__, fn.__name__, namespace)
         30 
    ---> 31     args = inspect.getargspec(fn)
         32     has_self = args[0] and args[0][0] in ('self', 'cls')
         33 
    
    /usr/lib64/python3.6/inspect.py in getargspec(func)
       1043         getfullargspec(func)
       1044     if kwonlyargs or ann:
    -> 1045         raise ValueError("Function has keyword-only parameters or annotations"
       1046                          ", use getfullargspec() API which can support them")
       1047     return ArgSpec(args, varargs, varkw, defaults)
    
    ValueError: Function has keyword-only parameters or annotations, use getfullargspec() API which can support them
    
  2. Mark Heppner

    Nevermind, I see you have a value for 3.2. How about using .getargspec() for PY2 and 3.2, and using .signature() for 3.3+? That would support all of PY2 and PY3 while still only displaying the deprecation warning for 3.2.

  3. Michael Bayer repo owner

    using the correct function is not the hard part, it's having those new arguments added to be part of the cache key. there is currently function_key_generator which does not support keyword arguments at all (which is what your stack trace is), and there's also kwarg_function_key_generator which does. But neither know how to recognize keyword-only arguments. that has to be implemented appropriately for both. keyword-only arguments would likely continue to be not implemented by function_key_generator and would be supported by kwarg_function_key_generator.

  4. Mark Heppner

    Ah yes. I didn't see anything in the docs that mention it.

    I'm not that familiar with this code, so sorry if this is a bit naive, but couldn't a unified key generator be used instead? Something like this would give the same cache key, regardless of how the args or kwargs are invoked (Python 3 only):

    import inspect 
    
    def test(a1, a2, k1='k1', k2='k2', *args, **kwargs):
        return True
    
    sig = inspect.signature(test)
    
    # apply user-supplied values
    bound = sig.bind('test1', 'test2', 'overridden')
    
    # apply any default values for kwargs
    bound.apply_defaults()
    
    # assumes all the values have str() version
    cache_key = ' ' .join([key + '=' + str(value) for key, value in bound.arguments.items()])
    # 'a1=test1 a2=test2 k1=overridden k2=k2 args=() kwargs={}'
    

    Basically, just treat any arg or kwarg as a named value as part of the cache key. Obviously this would increase key sizes, but I think it would only affect memcache's limit of 250 bytes.

    I haven't tried it yet, but a Python 2 version might be more challenging.

    Edit there is a backport of .inspect(). Not sure how you feel about an additional dependency though.

  5. Michael Bayer repo owner

    well it has to work on Python 2 to be "unified", but overall the "no-kwarg" key generator has been there much longer than the kwargs one, so there are two separate versions for stability and backwards compatilibity reasons.

  6. Rudolf Cardinal

    As the error message suggests, this also applies to type-annotated functions, as in

    from dogpile.cache import make_region
    static_cache = make_region().configure('dogpile.cache.memory')
    
    @static_cache.cache_on_arguments()  # will raise ValueError
    def get_string() -> str:  # because of the annotation
        return "hello"
    

    Not critical, obviously, but type-checking is a generally Good Thing.

    Thank you for dogpile.cache, Mike; this looks excellent (as ever!).

    all the best, Rudolf.

  7. Rudolf Cardinal

    Just noticed some comment errors in my code: (a) the docstring for fkg_allowing_type_hints says it handles keyword arguments, but it doesn't, and (b) in kw_fkg_allowing_type_hints.generate_key, the labelling of normal named positional args and "leftover" *args is backwards (but the code is right, I think). My intention was that kw_fkg_allowing_type_hints is the general-purpose one that you can use everywhere without worrying about it.

  8. Michael Bayer repo owner

    it's fine to use __qualname__ to help solve the function / method problem, however "namespace" itself is not necessarily used in that way, it can be any kind of value / tuple / whatever used to store cache results under a certain prefix.

  9. Rudolf Cardinal

    Also possible to augment this to make the FKG distinguish class instances, so you can cache normal methods of a class in a per-instance way - here's the most general form:

    def kw_fkg_allowing_type_hints(
            namespace: Optional[str],
            fn: Callable,
            to_str: Callable[[Any], str] = repr) \
            -> Callable[[Callable], str]:
        """
        As for fkg_allowing_type_hints, but allowing keyword arguments.
    
        For kwargs passed in, we will build a dict of all argname (key) argvalue
        (values) including default args from the argspec and then alphabetize the
        list before generating the key.
    
        NOTE ALSO that once we have keyword arguments, we should be using repr(),
        because we need to distinguish
    
            kwargs = {'p': 'another', 'q': 'thing'}
            ... which compat.string_type will make into
                    p=another q=thing
            ... from
            kwargs = {'p': 'another q=thing'}
    
        Also modified to make the cached function unique per INSTANCE for normal
        methods of a class.
        """
    
        namespace = get_namespace(fn, namespace)
    
        sig = inspect.signature(fn)
        parameters = list(sig.parameters.values())  # convert from odict_values
        argnames = [p.name for p in parameters
                    if p.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD]
        has_self = bool(argnames and argnames[0] in ('self', 'cls'))
    
        if DEBUG_INTERNALS:
            log.debug(
                "At start of kw_fkg_allowing_type_hints: namespace={namespace},"
                "parameters=[{parameters}], argnames={argnames}, "
                "has_self={has_self}, fn={fn}".format(
                    namespace=namespace,
                    parameters=", ".join(repr_parameter(p) for p in parameters),
                    argnames=repr(argnames),
                    has_self=has_self,
                    fn=repr(fn),
                ))
    
        def generate_key(*args, **kwargs):
            as_kwargs = {}  # type: Dict[str, Any]
            loose_args = []  # type: List[Any]  # those captured by *args
            # 1. args: get the name as well.
            for idx, arg in enumerate(args):
                if idx >= len(argnames):
                    # positional argument to be scooped up with *args
                    loose_args.append(arg)
                else:
                    # normal plain positional argument
                    if has_self and idx == 0:  # "self" or "cls" initial argument
                        argvalue = hex(id(arg))
                    else:
                        argvalue = arg
                    as_kwargs[argnames[idx]] = argvalue
            # 1b. args with no name
            if loose_args:
                as_kwargs['*args'] = loose_args
                # '*args' is guaranteed not to be a parameter name in its own right
            # 2. kwargs
            as_kwargs.update(kwargs)
            # 3. default values
            for param in parameters:
                if param.default != inspect.Parameter.empty:
                    if param.name not in as_kwargs:
                        as_kwargs[param.name] = param.default
            # 4. sorted by name
            #    ... but also incorporating the name of the argument, because once
            #    we allow the arbitrary **kwargs format, order is no longer
            #    sufficient to discriminate
            #       fn(p="another", q="thing")
            #    from
            #       fn(r="another", s="thing")
            argument_values = ["{k}={v}".format(k=key, v=to_str(as_kwargs[key]))
                               for key in sorted(as_kwargs.keys())]
            key = namespace + '|' + " ".join(argument_values)
            if DEBUG_INTERNALS:
                log.debug("kw_fkg_allowing_type_hints.generate_key() -> " +
                          repr(key))
            return key
    
        return generate_key
    

    I have test code too if it's of use; essentially you can then do:

    class TestClass(object):
        def __init__(self, a: int = 200) -> None:
            self.a = a
    
        @mycache.cache_on_arguments(function_key_generator=plain_fkg)
        def oneparam(self, q: str) -> str:
            fn_called("CACHED FUNCTION TestClass.oneparam() CALLED")
            return "TestClass.oneparam: hello, " + q
    
        @property
        @mycache.cache_on_arguments(function_key_generator=kw_fkg)
        def prop(self) -> str:
            fn_called("CACHED PROPERTY TestClass.prop() CALLED")
            return "TestClass.prop: a={}".format(repr(self.a))
    

    ... whereas with the default system, the per-instance specificity is lost.

  10. Log in to comment