From e143650cac839a84847cc010d037534440ccacd9 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Thu, 17 Nov 2016 18:52:58 -0500 Subject: [PATCH 1/2] Add defaults support to enable deprecation Add support for using declared keyword argument values from both hookspecs and hookimpls. The current logic will inspect the hookimpl and, if it contains defaults, values will be first looked up from the caller provided data and if not defined will be taken from the hookspec's declared defaults. If the spec does not define defaults the value is taken from the hookimpl's defaults as is expected under normal function call semantics. Resolves #15 --- pluggy.py | 76 +++++++++++++++++++++++---------- testing/test_method_ordering.py | 6 +-- testing/test_multicall.py | 2 +- 3 files changed, 57 insertions(+), 27 deletions(-) diff --git a/pluggy.py b/pluggy.py index fa30fcdb..8057f76e 100644 --- a/pluggy.py +++ b/pluggy.py @@ -1,5 +1,6 @@ import sys import inspect +import copy import warnings __version__ = '0.5.0' @@ -274,10 +275,12 @@ def __init__(self, project_name, implprefix=None): self.trace = _TagTracer().get("pluginmanage") self.hook = _HookRelay(self.trace.root.get("hook")) self._implprefix = implprefix - self._inner_hookexec = lambda hook, methods, kwargs: \ - _MultiCall( - methods, kwargs, specopts=hook.spec_opts, hook=hook - ).execute() + self._inner_hookexec = lambda hook, methods, kwargs: _MultiCall( + methods, + kwargs, + firstresult=hook.spec.opts['firstresult'] if hook.spec else False, + hook=hook + ).execute() def _hookexec(self, hook, methods, kwargs): # called from all hookcaller instances. @@ -424,7 +427,7 @@ def _verify_hook(self, hook, hookimpl): (hookimpl.plugin_name, hook.name)) # positional arg checking - notinspec = set(hookimpl.argnames) - set(hook.argnames) + notinspec = set(hookimpl.argnames) - set(hook.spec.argnames) if notinspec: raise PluginValidationError( "Plugin %r for hook %r\nhookimpl definition: %s\n" @@ -517,8 +520,8 @@ def subset_hook_caller(self, name, remove_plugins): orig = getattr(self.hook, name) plugins_to_remove = [plug for plug in remove_plugins if hasattr(plug, name)] if plugins_to_remove: - hc = _HookCaller(orig.name, orig._hookexec, orig._specmodule_or_class, - orig.spec_opts) + hc = _HookCaller(orig.name, orig._hookexec, orig.spec.namespace, + orig.spec.opts) for hookimpl in (orig._wrappers + orig._nonwrappers): plugin = hookimpl.plugin if plugin not in plugins_to_remove: @@ -539,29 +542,43 @@ class _MultiCall(object): # so we can remove it soon, allowing to avoid the below recursion # in execute() and simplify/speed up the execute loop. - def __init__(self, hook_impls, kwargs, specopts={}, hook=None): - self.hook = hook + def __init__(self, hook_impls, kwargs, firstresult=False, hook=None): self.hook_impls = hook_impls self.caller_kwargs = kwargs # come from _HookCaller.__call__() self.caller_kwargs["__multicall__"] = self - self.specopts = hook.spec_opts if hook else specopts + self.firstresult = firstresult + self.hook = hook + self.spec = hook.spec if hook else None def execute(self): caller_kwargs = self.caller_kwargs self.results = results = [] - firstresult = self.specopts.get("firstresult") + firstresult = self.firstresult + spec = self.spec while self.hook_impls: hook_impl = self.hook_impls.pop() + implkwargs = hook_impl.kwargs try: args = [caller_kwargs[argname] for argname in hook_impl.argnames] + # get any caller provided kwargs declared in our + # hookimpl and fail over to the spec's value if provided + if implkwargs: + kwargs = copy.copy(implkwargs) + if spec: + kwargs.update(spec.kwargs) + + args += [caller_kwargs.get(argname, kwargs[argname]) + for argname in hook_impl.kwargnames] except KeyError: for argname in hook_impl.argnames: if argname not in caller_kwargs: raise HookCallError( "hook call must provide argument %r" % (argname,)) + if hook_impl.hookwrapper: return _wrapped_call(hook_impl.function(*args), self.execute) + res = hook_impl.function(*args) if res is not None: if firstresult: @@ -645,28 +662,23 @@ def __init__(self, name, hook_execute, specmodule_or_class=None, spec_opts=None) self._wrappers = [] self._nonwrappers = [] self._hookexec = hook_execute - self.argnames = None - self.kwargnames = None + self.spec = None + self._call_history = None if specmodule_or_class is not None: assert spec_opts is not None self.set_specification(specmodule_or_class, spec_opts) def has_spec(self): - return hasattr(self, "_specmodule_or_class") + return self.spec is not None def set_specification(self, specmodule_or_class, spec_opts): assert not self.has_spec() - self._specmodule_or_class = specmodule_or_class - specfunc = getattr(specmodule_or_class, self.name) - # get spec arg signature - argnames, self.kwargnames = varnames(specfunc) - self.argnames = ["__multicall__"] + list(argnames) - self.spec_opts = spec_opts + self.spec = HookSpec(specmodule_or_class, self.name, spec_opts) if spec_opts.get("historic"): self._call_history = [] def is_historic(self): - return hasattr(self, "_call_history") + return self._call_history is not None def _remove_plugin(self, plugin): def remove(wrappers): @@ -702,8 +714,8 @@ def __repr__(self): def __call__(self, **kwargs): assert not self.is_historic() - if self.argnames: - notincall = set(self.argnames) - set(['__multicall__']) - set( + if self.spec: + notincall = set(self.spec.argnames) - set(['__multicall__']) - set( kwargs.keys()) if notincall: warnings.warn( @@ -749,10 +761,28 @@ def _maybe_apply_history(self, method): proc(res[0]) +class HookSpec(object): + def __init__(self, namespace, name, hook_spec_opts): + self.namespace = namespace + self.function = function = getattr(namespace, name) + self.name = name + self.argnames, self.kwargnames = varnames(function) + self.kwargvalues = inspect.getargspec(function).defaults + self.kwargs = dict( + zip(self.kwargnames, self.kwargvalues) + ) if self.kwargvalues else {} + self.opts = hook_spec_opts + self.argnames = ["__multicall__"] + list(self.argnames) + + class HookImpl(object): def __init__(self, plugin, plugin_name, function, hook_impl_opts): self.function = function self.argnames, self.kwargnames = varnames(self.function) + self.kwargvalues = inspect.getargspec(function).defaults + self.kwargs = dict( + zip(self.kwargnames, self.kwargvalues) + ) if self.kwargvalues else {} self.plugin = plugin self.opts = hook_impl_opts self.plugin_name = plugin_name diff --git a/testing/test_method_ordering.py b/testing/test_method_ordering.py index 7c87702a..8b2b321a 100644 --- a/testing/test_method_ordering.py +++ b/testing/test_method_ordering.py @@ -163,9 +163,9 @@ def he_myhook3(arg1): pass pm.add_hookspecs(HookSpec) - assert not pm.hook.he_myhook1.spec_opts["firstresult"] - assert pm.hook.he_myhook2.spec_opts["firstresult"] - assert not pm.hook.he_myhook3.spec_opts["firstresult"] + assert not pm.hook.he_myhook1.spec.opts["firstresult"] + assert pm.hook.he_myhook2.spec.opts["firstresult"] + assert not pm.hook.he_myhook3.spec.opts["firstresult"] @pytest.mark.parametrize('name', ["hookwrapper", "optionalhook", "tryfirst", "trylast"]) diff --git a/testing/test_multicall.py b/testing/test_multicall.py index 7aff8c33..26aeee6a 100644 --- a/testing/test_multicall.py +++ b/testing/test_multicall.py @@ -22,7 +22,7 @@ def MC(methods, kwargs, firstresult=False): for method in methods: f = HookImpl(None, "", method, method.example_impl) hookfuncs.append(f) - return _MultiCall(hookfuncs, kwargs, {"firstresult": firstresult}) + return _MultiCall(hookfuncs, kwargs, firstresult=firstresult) def test_call_passing(): From 9fdd7f376607a4c67a2a9550333419b50bae331e Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Sat, 19 Nov 2016 12:46:47 -0500 Subject: [PATCH 2/2] Add a defaults precedence test Verify that defaults declared in hook impls and specs adhere to the lookup order: call provided value, hook spec default, and finally falling back to the spec's default value. --- testing/test_hookrelay.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/testing/test_hookrelay.py b/testing/test_hookrelay.py index 38c5e2ad..c713caa5 100644 --- a/testing/test_hookrelay.py +++ b/testing/test_hookrelay.py @@ -76,3 +76,32 @@ def hello(self, arg): pm.register(Plugin()) res = pm.hook.hello(arg=3) assert res == 4 + + +def test_defaults(pm): + """Verify that default keyword arguments can be declared on both specs + and impls. The default value look up precedence is up as follows: + - caller provided value + - hookspec default + - hookimpl default + """ + class Api: + @hookspec + def myhook(self, arg, kwarg="default"): + "A spec with a default" + + class Plugin: + @hookimpl + def myhook(self, arg, kwarg="my default"): + return kwarg + + pm.register(Plugin()) + + # with no spec registered + assert pm.hook.myhook(arg='yeah!')[0] == "my default" + assert pm.hook.myhook(arg='yeah!', kwarg='doggy')[0] == "doggy" + + # with spec registered + pm.add_hookspecs(Api) + assert pm.hook.myhook(arg='yeah!')[0] == "default" + assert pm.hook.myhook(arg='yeah!', kwarg='doggy')[0] == "doggy"