From fa1235f3ac02acfec33469157dffcf5d7c6b4ece Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 23 Oct 2026 16:41:57 +0000 Subject: [PATCH 01/16] Fix duplicate empty kwargs --- src/blueapi/worker/task.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index d4cf0e809..b6cf3207d 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -97,7 +97,14 @@ def _inner_prepare_params( kwargs.update(value) else: # Let the generated Pydantic model provide the default. - kwargs[name] = getattr(validated, name) + value = getattr(validated, name) + match parameter.kind: + case Parameter.VAR_KEYWORD: + kwargs.update(value) + case Parameter.VAR_POSITIONAL: + args.extend(value) + case _: + kwargs[name] = value return args, kwargs @@ -108,6 +115,7 @@ def do_task(self, ctx: BlueskyContext) -> None: ) plan = ctx.plan_functions[self.name] prepared_args, prepared_kwargs = self.prepare_params(ctx) + print(prepared_args, prepared_kwargs) ctx.run_engine.md.update(self.metadata) result = ctx.run_engine(plan(*prepared_args, **prepared_kwargs)) if isinstance(result, tuple): # pragma: no cover From 6b4256d0883506471500cefbe61424811d8dcc71 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 23 Oct 2026 16:43:23 +0000 Subject: [PATCH 02/16] Remove print --- src/blueapi/worker/task.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index b6cf3207d..6e0bcf0e0 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -115,7 +115,6 @@ def do_task(self, ctx: BlueskyContext) -> None: ) plan = ctx.plan_functions[self.name] prepared_args, prepared_kwargs = self.prepare_params(ctx) - print(prepared_args, prepared_kwargs) ctx.run_engine.md.update(self.metadata) result = ctx.run_engine(plan(*prepared_args, **prepared_kwargs)) if isinstance(result, tuple): # pragma: no cover From 7cb6b91d14cc39c6670d9b8a91930a5a0df14061 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 26 Oct 2026 10:22:19 +0000 Subject: [PATCH 03/16] Don't supply pydantic schema defaults, these are supplied by the plan. --- src/blueapi/worker/task.py | 51 ++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index 6e0bcf0e0..006683b0e 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -67,8 +67,7 @@ def _inner_prepare_params( sig = signature(func) bound = sig.bind_partial(*self.args, **self.params) - # Only validate explicitly supplied arguments. This allows Pydantic's - # default/default_factory to provide injected defaults. + # Validate explicitly supplied arguments and resolve device names. adapter = TypeAdapter(plan.model) validated = adapter.validate_python(bound.arguments) @@ -77,34 +76,32 @@ def _inner_prepare_params( for name, parameter in sig.parameters.items(): supplied = name in bound.arguments + injected = isinstance(parameter.default, str) - if supplied: - value = getattr(validated, name) + # Let Python apply ordinary defaults itself. + if not supplied and not injected: + continue - match parameter.kind: - case Parameter.POSITIONAL_ONLY: - args.append(value) - case Parameter.VAR_POSITIONAL: - args.extend(value) - case Parameter.POSITIONAL_OR_KEYWORD: - if name in self.params: - kwargs[name] = value - else: - args.append(value) - case Parameter.KEYWORD_ONLY: - kwargs[name] = value - case Parameter.VAR_KEYWORD: - kwargs.update(value) - else: - # Let the generated Pydantic model provide the default. - value = getattr(validated, name) - match parameter.kind: - case Parameter.VAR_KEYWORD: - kwargs.update(value) - case Parameter.VAR_POSITIONAL: - args.extend(value) - case _: + value = getattr(validated, name) + + match parameter.kind: + case Parameter.POSITIONAL_ONLY: + args.append(value) + + case Parameter.VAR_POSITIONAL: + args.extend(value) + + case Parameter.POSITIONAL_OR_KEYWORD: + if name in self.params or (not supplied and injected): kwargs[name] = value + else: + args.append(value) + + case Parameter.KEYWORD_ONLY: + kwargs[name] = value + + case Parameter.VAR_KEYWORD: + kwargs.update(value) return args, kwargs From 478192a3a24dc63e403859cf69907fbe94ef09c5 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 26 Oct 2026 10:22:34 +0000 Subject: [PATCH 04/16] Add test to test for this bug --- tests/unit_tests/worker/test_task_worker.py | 48 +++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index 8f0a1269c..2b06c8aa5 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -1127,3 +1127,51 @@ def positionals(one: int, two: int) -> MsgGenerator: task.prepare_params(context) assert tpe.value.errors[0]["msg"] == "multiple values for argument 'one'" + + +@pytest.mark.parametrize( + ("extra_args", "params", "extra_kwargs"), + [ + ([], {}, {}), + ([2], {}, {}), + ([2, 3], {}, {}), + ([], {"group": "test"}, {}), + ([], {"wait": True}, {}), + ([], {"group": "test", "wait": True}, {}), + ([], {}, {"custom": "value"}), + ([], {}, {"custom": "value", "another": 42}), + ([2, 3], {"group": "test", "wait": True}, {"custom": "value"}), + ], +) +def test_plan_with_args_and_other_parameters_and_kwargs( + fake_device: FakeDevice, + extra_args: list[int], + params: dict[str, Any], + extra_kwargs: dict[str, Any], +): + def args_with_parameters_and_kwargs( + obj: FakeDevice, + *args: Any, + group: str | None = None, + wait: bool = False, + **kwargs: Any, + ) -> MsgGenerator: + yield from [] + + context = BlueskyContext() + context.register_plan(args_with_parameters_and_kwargs) + context.register_device(fake_device) + + task = Task( + name="args_with_parameters_and_kwargs", + args=[fake_device.name, 1, *extra_args], + params={**params, **extra_kwargs}, + ) + + prepared_args, prepared_kwargs = task.prepare_params(context) + + expected_args = [fake_device, 1, *extra_args] + expected_kwargs = {**params, **extra_kwargs} + + assert prepared_args == expected_args + assert prepared_kwargs == expected_kwargs From 957a336f6d60123d568f0b68572c03f95d3984a5 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 26 Oct 2026 10:51:44 +0000 Subject: [PATCH 05/16] Update inject to maybe_device --- src/blueapi/worker/task.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index 006683b0e..3a54e83fb 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -76,10 +76,10 @@ def _inner_prepare_params( for name, parameter in sig.parameters.items(): supplied = name in bound.arguments - injected = isinstance(parameter.default, str) + maybe_device = isinstance(parameter.default, str) # Let Python apply ordinary defaults itself. - if not supplied and not injected: + if not supplied and not maybe_device: continue value = getattr(validated, name) @@ -92,7 +92,7 @@ def _inner_prepare_params( args.extend(value) case Parameter.POSITIONAL_OR_KEYWORD: - if name in self.params or (not supplied and injected): + if name in self.params or (not supplied and maybe_device): kwargs[name] = value else: args.append(value) From 5eac6355a043864d9b77d8749b59c87138669983 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 26 Oct 2026 11:14:57 +0000 Subject: [PATCH 06/16] Update to use kwargs over params --- tests/unit_tests/worker/test_task_worker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index 2b06c8aa5..c5050b15a 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -1130,7 +1130,7 @@ def positionals(one: int, two: int) -> MsgGenerator: @pytest.mark.parametrize( - ("extra_args", "params", "extra_kwargs"), + ("extra_args", "kwargs", "extra_kwargs"), [ ([], {}, {}), ([2], {}, {}), @@ -1146,7 +1146,7 @@ def positionals(one: int, two: int) -> MsgGenerator: def test_plan_with_args_and_other_parameters_and_kwargs( fake_device: FakeDevice, extra_args: list[int], - params: dict[str, Any], + kwargs: dict[str, Any], extra_kwargs: dict[str, Any], ): def args_with_parameters_and_kwargs( @@ -1165,13 +1165,13 @@ def args_with_parameters_and_kwargs( task = Task( name="args_with_parameters_and_kwargs", args=[fake_device.name, 1, *extra_args], - params={**params, **extra_kwargs}, + params={**kwargs, **extra_kwargs}, ) prepared_args, prepared_kwargs = task.prepare_params(context) expected_args = [fake_device, 1, *extra_args] - expected_kwargs = {**params, **extra_kwargs} + expected_kwargs = {**kwargs, **extra_kwargs} assert prepared_args == expected_args assert prepared_kwargs == expected_kwargs From 89f1c0babc49ae07c100c1161e4ed2f53e1eb44f Mon Sep 17 00:00:00 2001 From: oliwenmandiamond <136330507+oliwenmandiamond@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:01:08 +0000 Subject: [PATCH 07/16] Apply suggestion from @tpoliaw Co-authored-by: Peter Holloway --- src/blueapi/worker/task.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index 3a54e83fb..439b7d4bf 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -68,8 +68,7 @@ def _inner_prepare_params( bound = sig.bind_partial(*self.args, **self.params) # Validate explicitly supplied arguments and resolve device names. - adapter = TypeAdapter(plan.model) - validated = adapter.validate_python(bound.arguments) + validated = plan.model.model_validate(bound.arguments) args: list[Any] = [] kwargs: dict[str, Any] = {} From dd822cd2e337c9fc6090f00c722be73457721a8d Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 27 Oct 2026 10:38:24 +0000 Subject: [PATCH 08/16] Fix lint --- src/blueapi/worker/task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index 439b7d4bf..c29d71895 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -3,7 +3,7 @@ from inspect import Parameter, signature from typing import Any, Self, TypedDict -from pydantic import Field, TypeAdapter, ValidationError +from pydantic import Field, ValidationError from blueapi.core import BlueskyContext from blueapi.utils import BlueapiBaseModel From 64917f51fb839d2fe3c2ab69f890302756bbc3b2 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 27 Oct 2026 11:08:32 +0000 Subject: [PATCH 09/16] Match spacing --- src/blueapi/worker/task.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index c29d71895..22018b305 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -86,19 +86,15 @@ def _inner_prepare_params( match parameter.kind: case Parameter.POSITIONAL_ONLY: args.append(value) - case Parameter.VAR_POSITIONAL: args.extend(value) - case Parameter.POSITIONAL_OR_KEYWORD: if name in self.params or (not supplied and maybe_device): kwargs[name] = value else: args.append(value) - case Parameter.KEYWORD_ONLY: kwargs[name] = value - case Parameter.VAR_KEYWORD: kwargs.update(value) From 468db126c3d693e7249389d3dced13a49da2de3c Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 27 Oct 2026 13:33:07 +0000 Subject: [PATCH 10/16] Improve logic, add detailed doc string, add detailed tests --- src/blueapi/worker/task.py | 62 ++++++++++++++++++++- tests/unit_tests/worker/test_task_worker.py | 55 ++++++++++++------ 2 files changed, 96 insertions(+), 21 deletions(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index 22018b305..221cf1c41 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -1,16 +1,47 @@ +import dataclasses +import inspect import logging from collections.abc import Mapping from inspect import Parameter, signature from typing import Any, Self, TypedDict -from pydantic import Field, ValidationError +from pydantic import BaseModel, Field, ValidationError -from blueapi.core import BlueskyContext +from blueapi.core import BlueskyContext, is_bluesky_compatible_device_type from blueapi.utils import BlueapiBaseModel LOGGER = logging.getLogger(__name__) +def _is_injected_parameter(parameter: Parameter) -> bool: + """Determine whether a parameter's string default represents injection. + + The ``inject()`` helper returns a string containing the device name (or an + empty string for composites). However, ordinary parameters can also have + string defaults, so the default value alone cannot identify an injected + parameter. + + Check the annotation to distinguish Bluesky-compatible devices and + composite models (including dataclasses) from ordinary parameters. This + ensures that ordinary defaults are left to Python rather than being + passed explicitly to the plan. + """ + default = parameter.default + # If parameter default is not str, it is not injected. + if not isinstance(default, str): + return False + + annotation = parameter.annotation + return ( + is_bluesky_compatible_device_type(annotation) # Check if annotation is device + # Check for composite inject + or inspect.isclass(annotation) + and ( + issubclass(annotation, BaseModel) or (dataclasses.is_dataclass(annotation)) + ) + ) + + class Task(BlueapiBaseModel): """ Task that will run a plan @@ -61,6 +92,31 @@ def prepare_params( def _inner_prepare_params( self, ctx: BlueskyContext ) -> tuple[list[Any], Mapping[str, Any]]: + """Prepare positional and keyword arguments for a plan invocation. + + Bind the task's supplied arguments to the plan's signature, then validate + them using the plan's Pydantic model. This also resolves injected device + names to the corresponding devices in the context. + + Only explicitly supplied arguments and injected parameters are included + in the returned arguments. Ordinary default values are omitted so that + Python applies the defaults defined by the plan itself, rather than + overriding them with defaults from the Pydantic model. + + Arguments are placed in the positional or keyword collections according + to their parameter kinds, preserving their intended binding when the + plan is invoked. Positional-or-keyword parameters supplied by name, and + injected parameters that were not explicitly supplied, are passed as + keywords to avoid changing which parameters receive their values. + + Args: + ctx: The Bluesky context containing the registered plan and its + associated parameter model. + + Returns: + A tuple containing the positional arguments and keyword arguments + to pass to the plan. + """ plan = ctx.plans[self.name] func = ctx.plan_functions[self.name] @@ -75,7 +131,7 @@ def _inner_prepare_params( for name, parameter in sig.parameters.items(): supplied = name in bound.arguments - maybe_device = isinstance(parameter.default, str) + maybe_device = _is_injected_parameter(parameter) # Let Python apply ordinary defaults itself. if not supplied and not maybe_device: diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index c5050b15a..fcf13d4f3 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -1115,7 +1115,6 @@ def positional_only_plan(val: int, /, **devices: FakeDevice) -> MsgGenerator: def test_arg_in_args_and_kwargs(): - def positionals(one: int, two: int) -> MsgGenerator: yield from [] @@ -1130,24 +1129,25 @@ def positionals(one: int, two: int) -> MsgGenerator: @pytest.mark.parametrize( - ("extra_args", "kwargs", "extra_kwargs"), - [ - ([], {}, {}), - ([2], {}, {}), - ([2, 3], {}, {}), - ([], {"group": "test"}, {}), - ([], {"wait": True}, {}), - ([], {"group": "test", "wait": True}, {}), - ([], {}, {"custom": "value"}), - ([], {}, {"custom": "value", "another": 42}), - ([2, 3], {"group": "test", "wait": True}, {"custom": "value"}), - ], + "extra_args", + [[], [2], [2, 3]], + ids=["no_extra_args", "one_extra_arg", "two_extra_args"], +) +@pytest.mark.parametrize( + "keyword_only_params", + [{}, {"group": "test"}, {"wait": True}, {"group": "test", "wait": True}], + ids=["no_keyword_only_params", "with_group", "with_wait", "with_group_and_wait"], +) +@pytest.mark.parametrize( + "variadic_kwargs", + [{}, {"custom": "value"}, {"custom": "value", "another": 42}], + ids=["no_variadic_kwargs", "one_variadic_kwarg", "two_variadic_kwargs"], ) def test_plan_with_args_and_other_parameters_and_kwargs( fake_device: FakeDevice, extra_args: list[int], - kwargs: dict[str, Any], - extra_kwargs: dict[str, Any], + keyword_only_params: dict[str, Any], + variadic_kwargs: dict[str, Any], ): def args_with_parameters_and_kwargs( obj: FakeDevice, @@ -1165,13 +1165,32 @@ def args_with_parameters_and_kwargs( task = Task( name="args_with_parameters_and_kwargs", args=[fake_device.name, 1, *extra_args], - params={**kwargs, **extra_kwargs}, + params={**keyword_only_params, **variadic_kwargs}, ) - prepared_args, prepared_kwargs = task.prepare_params(context) expected_args = [fake_device, 1, *extra_args] - expected_kwargs = {**kwargs, **extra_kwargs} + expected_kwargs = {**keyword_only_params, **variadic_kwargs} + # We only expect to see values provided by user and replaced str device names with + # devices. assert prepared_args == expected_args assert prepared_kwargs == expected_kwargs + + +def test_string_default_is_not_treated_as_device_injection(): + def plan_with_default_str(arg: str = "primary") -> MsgGenerator: + yield from [] + + primary = FakeDevice("primary") + context = BlueskyContext() + context.register_device(primary) + context.register_plan(plan_with_default_str) + + task = Task(name="plan_with_default_str") + + args, kwargs = task.prepare_params(context) + # Defaults provided by plan. Checking that device with same name is not added + # by mistake as the default plan value. + assert args == [] + assert kwargs == {} From 7195986dc0895f43d49c39dd3f41abe68b60eb03 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 27 Oct 2026 13:42:45 +0000 Subject: [PATCH 11/16] Update test name --- tests/unit_tests/worker/test_task_worker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index fcf13d4f3..91e39759b 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -1143,13 +1143,13 @@ def positionals(one: int, two: int) -> MsgGenerator: [{}, {"custom": "value"}, {"custom": "value", "another": 42}], ids=["no_variadic_kwargs", "one_variadic_kwarg", "two_variadic_kwargs"], ) -def test_plan_with_args_and_other_parameters_and_kwargs( +def test_plan_args_with_keyword_only_params_and_variadic_kwargs( fake_device: FakeDevice, extra_args: list[int], keyword_only_params: dict[str, Any], variadic_kwargs: dict[str, Any], ): - def args_with_parameters_and_kwargs( + def args_with_keyword_only_params_and_variadic_kwargs( obj: FakeDevice, *args: Any, group: str | None = None, @@ -1159,11 +1159,11 @@ def args_with_parameters_and_kwargs( yield from [] context = BlueskyContext() - context.register_plan(args_with_parameters_and_kwargs) + context.register_plan(args_with_keyword_only_params_and_variadic_kwargs) context.register_device(fake_device) task = Task( - name="args_with_parameters_and_kwargs", + name="args_with_keyword_only_params_and_variadic_kwargs", args=[fake_device.name, 1, *extra_args], params={**keyword_only_params, **variadic_kwargs}, ) From 93e5f7f3cb29898e6c952a23833a8b83b79d7da9 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 27 Oct 2026 15:03:08 +0000 Subject: [PATCH 12/16] update parameter name --- src/blueapi/worker/task.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index 221cf1c41..6edf1e93e 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -131,10 +131,10 @@ def _inner_prepare_params( for name, parameter in sig.parameters.items(): supplied = name in bound.arguments - maybe_device = _is_injected_parameter(parameter) + injected = _is_injected_parameter(parameter) # Let Python apply ordinary defaults itself. - if not supplied and not maybe_device: + if not supplied and not injected: continue value = getattr(validated, name) @@ -145,7 +145,7 @@ def _inner_prepare_params( case Parameter.VAR_POSITIONAL: args.extend(value) case Parameter.POSITIONAL_OR_KEYWORD: - if name in self.params or (not supplied and maybe_device): + if name in self.params or (not supplied and injected): kwargs[name] = value else: args.append(value) From adf28daa5197fbb4d6721ee094f90e91ad66d8a7 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 5 Nov 2026 15:42:03 +0000 Subject: [PATCH 13/16] Apply fix which keeps original bheaviour of pydantic model defaults --- src/blueapi/worker/task.py | 101 +++++++------------- tests/unit_tests/worker/test_task_worker.py | 7 +- 2 files changed, 39 insertions(+), 69 deletions(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index 6edf1e93e..e38a2018e 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -1,47 +1,16 @@ -import dataclasses -import inspect import logging from collections.abc import Mapping from inspect import Parameter, signature from typing import Any, Self, TypedDict -from pydantic import BaseModel, Field, ValidationError +from pydantic import Field, ValidationError -from blueapi.core import BlueskyContext, is_bluesky_compatible_device_type +from blueapi.core import BlueskyContext from blueapi.utils import BlueapiBaseModel LOGGER = logging.getLogger(__name__) -def _is_injected_parameter(parameter: Parameter) -> bool: - """Determine whether a parameter's string default represents injection. - - The ``inject()`` helper returns a string containing the device name (or an - empty string for composites). However, ordinary parameters can also have - string defaults, so the default value alone cannot identify an injected - parameter. - - Check the annotation to distinguish Bluesky-compatible devices and - composite models (including dataclasses) from ordinary parameters. This - ensures that ordinary defaults are left to Python rather than being - passed explicitly to the plan. - """ - default = parameter.default - # If parameter default is not str, it is not injected. - if not isinstance(default, str): - return False - - annotation = parameter.annotation - return ( - is_bluesky_compatible_device_type(annotation) # Check if annotation is device - # Check for composite inject - or inspect.isclass(annotation) - and ( - issubclass(annotation, BaseModel) or (dataclasses.is_dataclass(annotation)) - ) - ) - - class Task(BlueapiBaseModel): """ Task that will run a plan @@ -96,25 +65,19 @@ def _inner_prepare_params( Bind the task's supplied arguments to the plan's signature, then validate them using the plan's Pydantic model. This also resolves injected device - names to the corresponding devices in the context. - - Only explicitly supplied arguments and injected parameters are included - in the returned arguments. Ordinary default values are omitted so that - Python applies the defaults defined by the plan itself, rather than - overriding them with defaults from the Pydantic model. + names to the corresponding devices in the context and applies parameter + defaults defined by the model. - Arguments are placed in the positional or keyword collections according - to their parameter kinds, preserving their intended binding when the - plan is invoked. Positional-or-keyword parameters supplied by name, and - injected parameters that were not explicitly supplied, are passed as - keywords to avoid changing which parameters receive their values. + Construct the positional and keyword arguments according to the plan's + parameter kinds, ensuring that positional-only, variadic, and + keyword-only parameters are handled correctly. Args: - ctx: The Bluesky context containing the registered plan and its - associated parameter model. + ctx: The context containing the registered plan and its associated + parameter model. Returns: - A tuple containing the positional arguments and keyword arguments + A tuple containing the prepared positional and keyword arguments to pass to the plan. """ plan = ctx.plans[self.name] @@ -131,28 +94,34 @@ def _inner_prepare_params( for name, parameter in sig.parameters.items(): supplied = name in bound.arguments - injected = _is_injected_parameter(parameter) - - # Let Python apply ordinary defaults itself. - if not supplied and not injected: - continue - value = getattr(validated, name) + if supplied: + value = getattr(validated, name) - match parameter.kind: - case Parameter.POSITIONAL_ONLY: - args.append(value) - case Parameter.VAR_POSITIONAL: - args.extend(value) - case Parameter.POSITIONAL_OR_KEYWORD: - if name in self.params or (not supplied and injected): - kwargs[name] = value - else: + match parameter.kind: + case Parameter.POSITIONAL_ONLY: args.append(value) - case Parameter.KEYWORD_ONLY: - kwargs[name] = value - case Parameter.VAR_KEYWORD: - kwargs.update(value) + case Parameter.VAR_POSITIONAL: + args.extend(value) + case Parameter.POSITIONAL_OR_KEYWORD: + if name in self.params: + kwargs[name] = value + else: + args.append(value) + case Parameter.KEYWORD_ONLY: + kwargs[name] = value + case Parameter.VAR_KEYWORD: + kwargs.update(value) + else: + kwargs[name] = getattr(validated, name) + value = getattr(validated, name) + match parameter.kind: + case Parameter.VAR_KEYWORD: + kwargs.update(value) + case Parameter.VAR_POSITIONAL: + args.extend(value) + case _: + kwargs[name] = value return args, kwargs diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index 91e39759b..7c2e494e6 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -1190,7 +1190,8 @@ def plan_with_default_str(arg: str = "primary") -> MsgGenerator: task = Task(name="plan_with_default_str") args, kwargs = task.prepare_params(context) - # Defaults provided by plan. Checking that device with same name is not added - # by mistake as the default plan value. assert args == [] - assert kwargs == {} + # Defaults provided by plan. Checking that device with same name is not added + # by mistake as the default plan value. The kwargs is provided by pydantic model + # from schema. + assert kwargs == {"arg": "primary"} From 42f8a65e42ac5428c24188a14507e3203291e7b1 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 5 Nov 2026 16:22:44 +0000 Subject: [PATCH 14/16] Fix tests and bug --- src/blueapi/worker/task.py | 41 +++++++-------------- tests/unit_tests/worker/test_task_worker.py | 7 +++- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index e38a2018e..a86a0d1f9 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -93,35 +93,22 @@ def _inner_prepare_params( kwargs: dict[str, Any] = {} for name, parameter in sig.parameters.items(): - supplied = name in bound.arguments - - if supplied: - value = getattr(validated, name) - - match parameter.kind: - case Parameter.POSITIONAL_ONLY: + value = getattr(validated, name) + + match parameter.kind: + case Parameter.POSITIONAL_ONLY: + args.append(value) + case Parameter.VAR_POSITIONAL: + args.extend(value) + case Parameter.VAR_KEYWORD: + kwargs.update(value) + case Parameter.POSITIONAL_OR_KEYWORD: + if name in bound.arguments and name not in self.params: args.append(value) - case Parameter.VAR_POSITIONAL: - args.extend(value) - case Parameter.POSITIONAL_OR_KEYWORD: - if name in self.params: - kwargs[name] = value - else: - args.append(value) - case Parameter.KEYWORD_ONLY: - kwargs[name] = value - case Parameter.VAR_KEYWORD: - kwargs.update(value) - else: - kwargs[name] = getattr(validated, name) - value = getattr(validated, name) - match parameter.kind: - case Parameter.VAR_KEYWORD: - kwargs.update(value) - case Parameter.VAR_POSITIONAL: - args.extend(value) - case _: + else: kwargs[name] = value + case Parameter.KEYWORD_ONLY: + kwargs[name] = value return args, kwargs diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index 7c2e494e6..a23e39d35 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -1170,7 +1170,12 @@ def args_with_keyword_only_params_and_variadic_kwargs( prepared_args, prepared_kwargs = task.prepare_params(context) expected_args = [fake_device, 1, *extra_args] - expected_kwargs = {**keyword_only_params, **variadic_kwargs} + expected_kwargs = { + "group": None, # Defaults supplied by schema from registering plan. + "wait": False, # Defaults supplied by schema from registering plan. + **keyword_only_params, + **variadic_kwargs, + } # We only expect to see values provided by user and replaced str device names with # devices. From e2fa67418645e219c6ca199eecd4a02037201a59 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 5 Nov 2026 16:25:08 +0000 Subject: [PATCH 15/16] Remove outdated comment --- tests/unit_tests/worker/test_task_worker.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index a23e39d35..044668d93 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -1176,9 +1176,6 @@ def args_with_keyword_only_params_and_variadic_kwargs( **keyword_only_params, **variadic_kwargs, } - - # We only expect to see values provided by user and replaced str device names with - # devices. assert prepared_args == expected_args assert prepared_kwargs == expected_kwargs From 47d0af8e3672c862b8855614694404b29b99daa3 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 5 Nov 2026 16:29:01 +0000 Subject: [PATCH 16/16] Remove another outdated comment --- src/blueapi/worker/task.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index a86a0d1f9..0c76542fa 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -86,7 +86,6 @@ def _inner_prepare_params( sig = signature(func) bound = sig.bind_partial(*self.args, **self.params) - # Validate explicitly supplied arguments and resolve device names. validated = plan.model.model_validate(bound.arguments) args: list[Any] = []