From 09868be9d2db3c87e8b6552cd552c9f0fdfaac6a Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 22 Sep 2026 22:37:24 +0300 Subject: [PATCH 1/4] benchmark: increase test_hook_and_wrappers_speed rounds 10 has too much noise. --- testing/benchmark.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/benchmark.py b/testing/benchmark.py index 0ca52ad2..f57fe05e 100644 --- a/testing/benchmark.py +++ b/testing/benchmark.py @@ -112,7 +112,7 @@ def setup(): firstresult = False return (hook_name, hook_impls, caller_kwargs, firstresult), {} - benchmark.pedantic(_multicall, setup=setup, rounds=10) + benchmark.pedantic(_multicall, setup=setup, rounds=100) @pytest.mark.parametrize( From 8df74a155808840c5c49d4dbf666f7a314756d8c Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 22 Sep 2026 23:07:42 +0300 Subject: [PATCH 2/4] docs: add missing versionadded for hookspec argument defaults --- docs/index.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/index.rst b/docs/index.rst index 9d56f019..ca1f7852 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -640,6 +640,8 @@ callers: # New caller; hookimpls will get new_arg="get this". pm.hook.myhook(config=config, args=args, new_arg="get this") +.. versionadded:: 1.7 + Hookspec argument defaults. .. _firstresult: From 59bab5b63d72d9c05a3e5a3aae04e5eda605a1d7 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 23 Sep 2026 16:24:55 +0300 Subject: [PATCH 3/4] hooks: improve docstring for HookImpl.kwargnames --- src/pluggy/_hooks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pluggy/_hooks.py b/src/pluggy/_hooks.py index 3c1eaaaa..aa5858eb 100644 --- a/src/pluggy/_hooks.py +++ b/src/pluggy/_hooks.py @@ -705,9 +705,10 @@ def __init__( #: The hook implementation function. self.function: Final = function argnames, kwargnames = varnames(self.function) - #: The positional parameter names of ``function```. + #: The positional parameter names of ``function``. self.argnames: Final = argnames - #: The keyword parameter names of ``function```. + #: The keyword parameter names of ``function`` which declare defaults. + #: Keyword-only parameters are *not* included. self.kwargnames: Final = kwargnames #: The plugin which defined this hook implementation. self.plugin: Final = plugin From 51ec4486b9d47da944725b0326a412cf9b7518bf Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 22 Sep 2026 22:57:06 +0300 Subject: [PATCH 4/4] Change precedence of hook call value, hookspec default, hookimpl default Fix #442. --- changelog/442.bugfix.rst | 4 ++ docs/index.rst | 23 ++++++++++++ src/pluggy/_callers.py | 35 +++++++++++++++--- testing/benchmark.py | 30 +++++++++++++++ testing/test_invocations.py | 73 +++++++++++++++++++++++++++++++++---- 5 files changed, 151 insertions(+), 14 deletions(-) create mode 100644 changelog/442.bugfix.rst diff --git a/changelog/442.bugfix.rst b/changelog/442.bugfix.rst new file mode 100644 index 00000000..26c4caa9 --- /dev/null +++ b/changelog/442.bugfix.rst @@ -0,0 +1,4 @@ +With the addition of default values for hookspec arguments, we also changed the precedence of default values for *hookimpl* arguments. +Previously, if a hookimpl function declared a default for an argument, it was always used, even if the *hook call* passed an explicit value. +Now the precedence is changed to: hook call passed value > hookspec default value > hookimpl default value. +See :ref:`hookimpl_arg_defaults`. diff --git a/docs/index.rst b/docs/index.rst index ca1f7852..3768e54c 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -502,6 +502,29 @@ method. :py:meth:`~pluggy.Result.force_exception` to adjust the exception. +.. _hookimpl_arg_defaults: + +Hookimpl argument defaults +^^^^^^^^^^^^^^^^^^^^^^^^^^ +A hookimpl can declare default values for its arguments, as in ``version=1`` in +this example plugin:: + + class CompatiblePlugin: + @hookimpl + def setup_project(self, config, args, version=1): + ... + +This can be helpful if the host application evolved the hookspec to add a new +argument, but the plugin still wants to support an old version of the host +application which didn't pass this argument. + +If the hook caller passes a value for the parameter, it is used. Otherwise the +default declared by the hookimpl is used. + +.. versionchanged:: 1.7 + Previous versions had a different, unhelpful, behavior of *always* using the + hookimpl-declared default. + .. _specs: Specifications diff --git a/src/pluggy/_callers.py b/src/pluggy/_callers.py index 8b4b1477..974e2fb2 100644 --- a/src/pluggy/_callers.py +++ b/src/pluggy/_callers.py @@ -25,15 +25,22 @@ def run_old_style_hookwrapper( - hook_impl: HookImpl, hook_name: str, args: Sequence[object] + hook_impl: HookImpl, + hook_name: str, + args: Sequence[object], + kwargs: Mapping[str, object] | None, ) -> Teardown: """ backward compatibility wrapper to run a old style hookwrapper as a wrapper """ + if kwargs is None: + res = hook_impl.function(*args) + else: + res = hook_impl.function(*args, **kwargs) if TYPE_CHECKING: - teardown = cast(Teardown, hook_impl.function(*args)) + teardown = cast(Teardown, res) else: - teardown = hook_impl.function(*args) + teardown = res try: next(teardown) except StopIteration: @@ -98,19 +105,32 @@ def _multicall( for hook_impl in reversed(hook_impls): try: args = [caller_kwargs[argname] for argname in hook_impl.argnames] + if hook_impl.kwargnames: + kwargs = { + argname: caller_kwargs[argname] + for argname in hook_impl.kwargnames + if argname in caller_kwargs + } or None + else: + kwargs = None except KeyError as e: raise HookCallError( f"hook call must provide argument {e.args[0]!r}" ) from e if hook_impl.hookwrapper: - function_gen = run_old_style_hookwrapper(hook_impl, hook_name, args) + function_gen = run_old_style_hookwrapper( + hook_impl, hook_name, args, kwargs + ) next(function_gen) # first yield teardowns.append(function_gen) elif hook_impl.wrapper: - res = hook_impl.function(*args) + if kwargs is None: + res = hook_impl.function(*args) + else: + res = hook_impl.function(*args, **kwargs) # If this cast is not valid, a type error is raised below, # which is the desired response. if TYPE_CHECKING: @@ -123,7 +143,10 @@ def _multicall( _raise_wrapfail(function_gen, "did not yield") teardowns.append(function_gen) else: - res = hook_impl.function(*args) + if kwargs is None: + res = hook_impl.function(*args) + else: + res = hook_impl.function(*args, **kwargs) if res is not None: results.append(res) if firstresult: # halt further impl calls diff --git a/testing/benchmark.py b/testing/benchmark.py index f57fe05e..b5bb1879 100644 --- a/testing/benchmark.py +++ b/testing/benchmark.py @@ -91,6 +91,11 @@ def wrapper(arg1, arg2, arg3): return (yield) +@hookimpl +def hook_with_default_argument(arg1, arg2=2): + return arg1, arg2 + + @pytest.fixture(params=[10, 100], ids="hooks={}".format) def hooks(request: Any) -> list[object]: return [hook for i in range(request.param)] @@ -115,6 +120,31 @@ def setup(): benchmark.pedantic(_multicall, setup=setup, rounds=100) +@pytest.mark.parametrize("impl_count", [10, 100]) +@pytest.mark.parametrize( + "pass_arg", [False, True], ids=["impl-default", "caller-value"] +) +def test_hookimpl_with_default_speed( + benchmark, impl_count: int, pass_arg: bool +) -> None: + def setup(): + hook_impls = [ + HookImpl( + None, + "", + hook_with_default_argument, + hook_with_default_argument.example_impl, # type: ignore[attr-defined] + ) + for _ in range(impl_count) + ] + caller_kwargs = {"arg1": 1} + if pass_arg: + caller_kwargs["arg2"] = 2 + return ("foo", hook_impls, caller_kwargs, False), {} + + benchmark.pedantic(_multicall, setup=setup, rounds=100) + + @pytest.mark.parametrize( ("plugins, wrappers, nesting"), [ diff --git a/testing/test_invocations.py b/testing/test_invocations.py index f67b0eb9..ccae8aea 100644 --- a/testing/test_invocations.py +++ b/testing/test_invocations.py @@ -446,15 +446,72 @@ def hello(self, arg, new_arg="impl-default"): pm.register(PluginWithoutImplDefault()) pm.register(PluginWithImplDefault()) + # Old spec, new impl, old call. + # The normal case for impl default. assert pm.hook.hello(arg=1) == [(1, "impl-default")] + # Old spec, new impl, new call. + # Less common, but the call arg is still passed. + assert pm.hook.hello(arg=1, new_arg="call") == [(1, "call")] - def test_does_not_override_hookimpl_default(self, pm: PluginManager) -> None: - """If an impl provides its own default, it takes precedence over both - the spec default and call value. + def test_call_value_overrides_hookimpl_default(self, pm: PluginManager) -> None: + """A call value takes precedence over an hookimpl default.""" - NOTE: This is verifying existing behavior, but it's not necessarily what - we want (#442). - """ + class Api: + @hookspec + def hello(self, arg, new_arg): + pass + + class Plugin: + @hookimpl + def hello(self, arg, new_arg="impl-default"): + return new_arg + + pm.add_hookspecs(Api) + pm.register(Plugin()) + + assert pm.hook.hello(arg=1, new_arg="call") == ["call"] + + def test_call_value_overrides_hookimpl_default_in_wrappers( + self, pm: PluginManager + ) -> None: + class Api: + @hookspec + def hello(self, arg, new_arg): + pass + + seen = [] + + class Plugin: + @hookimpl + def hello(self, arg, new_arg="impl-default"): + seen.append(new_arg) + return new_arg + + class WrapperPlugin: + @hookimpl(wrapper=True) + def hello(self, arg, new_arg="impl-default"): + seen.append(new_arg) + result = yield + return result + + class HookwrapperPlugin: + @hookimpl(hookwrapper=True) + def hello(self, arg, new_arg="impl-default"): + seen.append(new_arg) + yield + + pm.add_hookspecs(Api) + pm.register(Plugin()) + pm.register(WrapperPlugin()) + pm.register(HookwrapperPlugin()) + + assert pm.hook.hello(arg=1, new_arg="call") == ["call"] + assert seen == ["call", "call", "call"] + + def test_hookspec_default_and_call_override_hookimpl_default( + self, pm: PluginManager + ) -> None: + """Hookspec defaults take precedence over hookimpl defaults.""" class Api: @hookspec @@ -469,8 +526,8 @@ def hello(self, arg, new_arg="impl-default"): pm.add_hookspecs(Api) pm.register(Plugin()) - assert pm.hook.hello(arg=1) == ["impl-default"] - assert pm.hook.hello(arg=1, new_arg="call") == ["impl-default"] + assert pm.hook.hello(arg=1) == ["spec-default"] + assert pm.hook.hello(arg=1, new_arg="call") == ["call"] def test_new_call_argument_is_not_delivered_by_old_spec( self, pm: PluginManager