support python3 keyword-only arguments for function decorator

[sqlalchemy-bot]

Migrated issue, originally created by Anonymous

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

[sqlalchemy-bot]

Mark Heppner (mheppner) wrote:

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

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

Python3 style keyword-only parameters arent' supported as of yet. if someone wants to work on a PR for this feature that is fine.

[sqlalchemy-bot]

Changes by Michael Bayer (zzzeek):

• edited description

[sqlalchemy-bot]

Changes by Michael Bayer (zzzeek):

• changed title from "Error in python 3.5 with use of deprecated getargs" to "support python3 keyword-only arguments for functio"

[sqlalchemy-bot]

Mark Heppner (mheppner) wrote:

I'm trying to figure out how to do this...

Does this seem acceptable to you? If you ignore 3.2, you can either use .getargspec() for PY2 or .signature() for PY3 (excluding 3.2).

[sqlalchemy-bot]

Mark Heppner (mheppner) wrote:

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.

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

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.

[sqlalchemy-bot]

Mark Heppner (mheppner) wrote:

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.

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

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.

[sqlalchemy-bot]

Mark Heppner (mheppner) wrote:

I didn't link it correctly, but here's an initial idea.

[sqlalchemy-bot]

Rudolf Cardinal (RudolfCardinal) wrote:

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.

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

I think we're still open to contributors on this one.

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

that looks extremely useful thanks!

[sqlalchemy-bot]

Rudolf Cardinal (RudolfCardinal) wrote:

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.

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

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.

[sqlalchemy-bot]

Rudolf Cardinal (RudolfCardinal) wrote:

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:

#!python

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:

#!python

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.

[zzzeek]

I haven't worked with type annotations. that looks like a lot of code, if we can get it into a PR here or a gerrit with some tests I can try to get people to help me look at it