From 1467fc644ecf37c4dba3afaf3d18d21d91906f9e Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Apr 2026 15:28:58 +0100 Subject: [PATCH 01/13] Imrpove BlueapiClient to add plan parameter type hints --- Pipfile | 11 ++++++ src/blueapi/client/client.py | 74 +++++++++++++++++++++++++++++++++--- 2 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 Pipfile diff --git a/Pipfile b/Pipfile new file mode 100644 index 0000000000..0757494bb3 --- /dev/null +++ b/Pipfile @@ -0,0 +1,11 @@ +[[source]] +url = "https://pypi.org/simple" +verify_ssl = true +name = "pypi" + +[packages] + +[dev-packages] + +[requires] +python_version = "3.11" diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index c5b41ff45e..ff16c68a65 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -6,7 +6,8 @@ from functools import cached_property from itertools import chain from pathlib import Path -from typing import Self +from textwrap import dedent, indent +from typing import Any, Self from bluesky_stomp.messaging import MessageContext, StompClient from bluesky_stomp.models import Broker @@ -50,6 +51,37 @@ log = logging.getLogger(__name__) +def _pretty_type(schema: dict[str, Any]) -> str: + # refs first + if "$ref" in schema: + return schema["$ref"].split("/")[-1] + + # arrays preserve inner type + if schema.get("type") == "array": + item_schema = schema.get("items", {}) + inner = _pretty_type(item_schema) + return f"list[{inner}]" + + # unions + if "anyOf" in schema: + return " | ".join(_pretty_type(s) for s in schema["anyOf"]) + + json_type = schema.get("type") + + type_map = { + "string": "str", + "integer": "int", + "boolean": "bool", + "number": "float", + "object": "dict", + } + + if isinstance(json_type, str): + return type_map.get(json_type, json_type.split(".")[-1]) + + return "Any" + + class MissingInstrumentSessionError(Exception): pass @@ -154,7 +186,7 @@ def help_text(self) -> str: return self.model.description or f"Plan {self!r}" @property - def properties(self) -> set[str]: + def properties(self) -> dict[str, Any]: return self.model.parameter_schema.get("properties", {}).keys() @property @@ -192,9 +224,18 @@ def _build_args(self, *args, **kwargs): return params def __repr__(self): - opts = [p for p in self.properties if p not in self.required] - params = ", ".join(chain(self.required, (f"{opt}=None" for opt in opts))) - return f"{self.name}({params})" + props = self.model.parameter_schema.get("properties", {}) + tab = " " + args = [] + for name, info in props.items(): + typ = _pretty_type(info) + arg = f"{tab}{name}: {typ}" + if name not in self.required: + arg = f"{arg} | None = None" + args.append(arg) + + joined = ",\n".join(args) + return f"{self.name}(\n{joined}\n)" class BlueapiClient: @@ -281,6 +322,29 @@ def get_plans(self) -> PlanResponse: """ return self._rest.get_plans() + @property + def ls_plans(self): + for plan in self.plans: + print(plan) + print(plan.help_text) + # print("\n") + # tab = " " + + # for plan in self.get_plans().plans: + # self.ls_plan(plan.name) + + # def ls_plan(self, name: str): + # tab = " " + # plan = self.get_plan(name) + # name_with_signature = schema_to_signature(plan.parameter_schema, plan.name) + # print(name_with_signature) + # # print(schema_to_signature(plan.model_json_schema())) + # if plan.description is not None: + # print(indent(dedent(plan.description).strip(), tab)) + # # print(plan.parameter_schema) + # else: + # print(indent("No documentation provided.", tab)) + @start_as_current_span(TRACER, "name") @deprecated("plans[name]") def get_plan(self, name: str) -> PlanModel: From d67adf820a24f3f4ae7b420ef6fd61d2e368aa77 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Apr 2026 15:30:09 +0100 Subject: [PATCH 02/13] Remove Pipfile --- Pipfile | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 Pipfile diff --git a/Pipfile b/Pipfile deleted file mode 100644 index 0757494bb3..0000000000 --- a/Pipfile +++ /dev/null @@ -1,11 +0,0 @@ -[[source]] -url = "https://pypi.org/simple" -verify_ssl = true -name = "pypi" - -[packages] - -[dev-packages] - -[requires] -python_version = "3.11" From ff420433c883b996598d47e160c35d14aba6132d Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Apr 2026 15:30:46 +0100 Subject: [PATCH 03/13] Remove unused code --- src/blueapi/client/client.py | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index ff16c68a65..eb1f0bfefa 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -4,9 +4,7 @@ from collections.abc import Iterable from concurrent.futures import Future from functools import cached_property -from itertools import chain from pathlib import Path -from textwrap import dedent, indent from typing import Any, Self from bluesky_stomp.messaging import MessageContext, StompClient @@ -322,29 +320,6 @@ def get_plans(self) -> PlanResponse: """ return self._rest.get_plans() - @property - def ls_plans(self): - for plan in self.plans: - print(plan) - print(plan.help_text) - # print("\n") - # tab = " " - - # for plan in self.get_plans().plans: - # self.ls_plan(plan.name) - - # def ls_plan(self, name: str): - # tab = " " - # plan = self.get_plan(name) - # name_with_signature = schema_to_signature(plan.parameter_schema, plan.name) - # print(name_with_signature) - # # print(schema_to_signature(plan.model_json_schema())) - # if plan.description is not None: - # print(indent(dedent(plan.description).strip(), tab)) - # # print(plan.parameter_schema) - # else: - # print(indent("No documentation provided.", tab)) - @start_as_current_span(TRACER, "name") @deprecated("plans[name]") def get_plan(self, name: str) -> PlanModel: From 73458170501e0210e7ed7d2acffe24b09d40ef8d Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Apr 2026 15:32:00 +0100 Subject: [PATCH 04/13] Clean up formatting --- src/blueapi/client/client.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index eb1f0bfefa..6564f20989 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -50,22 +50,18 @@ def _pretty_type(schema: dict[str, Any]) -> str: - # refs first if "$ref" in schema: return schema["$ref"].split("/")[-1] - # arrays preserve inner type if schema.get("type") == "array": item_schema = schema.get("items", {}) inner = _pretty_type(item_schema) return f"list[{inner}]" - # unions if "anyOf" in schema: return " | ".join(_pretty_type(s) for s in schema["anyOf"]) json_type = schema.get("type") - type_map = { "string": "str", "integer": "int", @@ -73,7 +69,6 @@ def _pretty_type(schema: dict[str, Any]) -> str: "number": "float", "object": "dict", } - if isinstance(json_type, str): return type_map.get(json_type, json_type.split(".")[-1]) From ec97173d2ba9ac9e7d787b1d17776c4cffae42ab Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Apr 2026 15:50:57 +0100 Subject: [PATCH 05/13] Add single and multi line support --- src/blueapi/client/client.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index 6564f20989..7ef94ba764 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -222,13 +222,21 @@ def __repr__(self): args = [] for name, info in props.items(): typ = _pretty_type(info) - arg = f"{tab}{name}: {typ}" + arg = f"{name}: {typ}" if name not in self.required: arg = f"{arg} | None = None" args.append(arg) - joined = ",\n".join(args) - return f"{self.name}(\n{joined}\n)" + single_line = f"{self.name}({', '.join(args)})" + max_length = 100 + max_args_inline = 4 + if len(single_line) <= max_length and len(args) <= max_args_inline: + return single_line + + # Fall back to multiline if too many arguments or too long. + multiline_args = ",\n".join(f"{tab}{arg}" for arg in args) + + return f"{self.name}(\n{multiline_args}\n)" class BlueapiClient: From a1ba5d44487074bfee158f63103e547465752f6e Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Apr 2026 16:04:57 +0100 Subject: [PATCH 06/13] Fix tests + add additional test --- src/blueapi/client/client.py | 2 +- tests/unit_tests/client/test_client.py | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index 7ef94ba764..d62b97e1ae 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -229,7 +229,7 @@ def __repr__(self): single_line = f"{self.name}({', '.join(args)})" max_length = 100 - max_args_inline = 4 + max_args_inline = 3 if len(single_line) <= max_length and len(args) <= max_args_inline: return single_line diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index a96f428e8c..cd28c74203 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -716,7 +716,29 @@ def test_plan_fallback_help_text(client): ), client, ) - assert plan.help_text == "Plan foo(one, two=None)" + assert plan.help_text == "Plan foo(one: Any, two: Any | None = None)" + + +def test_plan_multi_parameter_fallback_help_text(client): + plan = Plan( + "foo", + PlanModel( + name="foo", + schema={ + "properties": {"one": {}, "two": {}, "three": {}, "four": {}}, + "required": ["one"], + }, + ), + client, + ) + assert ( + plan.help_text == "Plan foo(\n" + " one: Any,\n" + " two: Any | None = None,\n" + " three: Any | None = None,\n" + " four: Any | None = None\n" + ")" + ) def test_plan_properties(client): From a8aa80533eeac7ce22ee5d22261b1eca8b6bebcf Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Apr 2026 16:11:34 +0100 Subject: [PATCH 07/13] Add more tests --- tests/unit_tests/client/test_client.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index cd28c74203..f79b8aaefa 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -725,8 +725,15 @@ def test_plan_multi_parameter_fallback_help_text(client): PlanModel( name="foo", schema={ - "properties": {"one": {}, "two": {}, "three": {}, "four": {}}, - "required": ["one"], + "properties": { + "one": {}, + "two": { + "anyOf": [{"items": {}, "type": "array"}, {"type": "boolean"}], + }, + "three": {}, + "four": {}, + }, + "required": ["one", "two"], }, ), client, @@ -734,7 +741,7 @@ def test_plan_multi_parameter_fallback_help_text(client): assert ( plan.help_text == "Plan foo(\n" " one: Any,\n" - " two: Any | None = None,\n" + " two: list[Any] | bool,\n" " three: Any | None = None,\n" " four: Any | None = None\n" ")" From 17e86b0e59452b69e98b1d78de1bb2721f56d340 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 7 Apr 2026 10:28:03 +0100 Subject: [PATCH 08/13] Improve type hints for plan signature to include defaults --- src/blueapi/client/client.py | 15 +++++++++--- src/blueapi/core/context.py | 32 ++++++++------------------ tests/unit_tests/client/test_client.py | 8 +++---- tests/unit_tests/core/test_context.py | 27 ++++++++++++++-------- 4 files changed, 42 insertions(+), 40 deletions(-) diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index d62b97e1ae..6ce31d374d 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -218,24 +218,33 @@ def _build_args(self, *args, **kwargs): def __repr__(self): props = self.model.parameter_schema.get("properties", {}) + required = set(self.required) + tab = " " args = [] + for name, info in props.items(): typ = _pretty_type(info) arg = f"{name}: {typ}" - if name not in self.required: - arg = f"{arg} | None = None" + + if name not in required: + if "default" in info: + default = repr(info["default"]) + arg = f"{arg} = {default}" + else: + arg = f"{arg} | None = None" + args.append(arg) single_line = f"{self.name}({', '.join(args)})" max_length = 100 max_args_inline = 3 + if len(single_line) <= max_length and len(args) <= max_args_inline: return single_line # Fall back to multiline if too many arguments or too long. multiline_args = ",\n".join(f"{tab}{arg}" for arg in args) - return f"{self.name}(\n{multiline_args}\n)" diff --git a/src/blueapi/core/context.py b/src/blueapi/core/context.py index 78682ccf6e..2fbc7386c4 100644 --- a/src/blueapi/core/context.py +++ b/src/blueapi/core/context.py @@ -5,7 +5,7 @@ from importlib import import_module from inspect import Parameter, isclass, signature from types import ModuleType, NoneType, UnionType -from typing import Any, Generic, TypeVar, Union, get_args, get_origin, get_type_hints +from typing import Any, TypeVar, Union, get_args, get_origin, get_type_hints from bluesky.protocols import HasName from bluesky.run_engine import RunEngine @@ -516,14 +516,16 @@ def _type_spec_for_function( ): default_factory = self._composite_factory(arg_type) _type = SkipJsonSchema[self._convert_type(arg_type, no_default)] + field_info = FieldInfo(default_factory=default_factory) else: - default_factory = DefaultFactory(para.default) _type = self._convert_type(arg_type, no_default) - factory = None if no_default else default_factory - new_args[name] = ( - _type, - FieldInfo(default_factory=factory), - ) + if no_default: + field_info = FieldInfo() + else: + field_info = FieldInfo(default=para.default) + + new_args[name] = (_type, field_info) + return new_args def _convert_type(self, typ: Any, no_default: bool = True) -> type: @@ -574,19 +576,3 @@ def _inject_composite(): return composite_class(**devices) return _inject_composite - - -D = TypeVar("D") - - -class DefaultFactory(Generic[D]): - _value: D - - def __init__(self, value: D): - self._value = value - - def __call__(self) -> D: - return self._value - - def __eq__(self, other) -> bool: - return other.__class__ == self.__class__ and self._value == other._value diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index f79b8aaefa..5a7842fd9f 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -730,8 +730,8 @@ def test_plan_multi_parameter_fallback_help_text(client): "two": { "anyOf": [{"items": {}, "type": "array"}, {"type": "boolean"}], }, - "three": {}, - "four": {}, + "three": {"default": 3}, + "four": {"default": None}, }, "required": ["one", "two"], }, @@ -742,8 +742,8 @@ def test_plan_multi_parameter_fallback_help_text(client): plan.help_text == "Plan foo(\n" " one: Any,\n" " two: list[Any] | bool,\n" - " three: Any | None = None,\n" - " four: Any | None = None\n" + " three: Any = 3,\n" + " four: Any = None\n" ")" ) diff --git a/tests/unit_tests/core/test_context.py b/tests/unit_tests/core/test_context.py index 738a4c564c..c089ed9873 100644 --- a/tests/unit_tests/core/test_context.py +++ b/tests/unit_tests/core/test_context.py @@ -46,7 +46,7 @@ TiledConfig, ) from blueapi.core import BlueskyContext, is_bluesky_compatible_device -from blueapi.core.context import DefaultFactory, generic_bounds, qualified_name +from blueapi.core.context import generic_bounds, qualified_name from blueapi.core.protocols import DeviceConnectResult, DeviceManager from blueapi.utils.connect_devices import _establish_device_connections from blueapi.utils.invalid_config_error import InvalidConfigError @@ -431,9 +431,9 @@ def test_with_config_passes_mock_to_with_dodal_module( def test_function_spec(empty_context: BlueskyContext): spec = empty_context._type_spec_for_function(has_some_params) assert spec["foo"][0] is int - assert spec["foo"][1].default_factory == DefaultFactory(42) + assert spec["foo"][1].default == 42 assert spec["bar"][0] is str - assert spec["bar"][1].default_factory == DefaultFactory("bar") + assert spec["bar"][1].default == "bar" def test_basic_type_conversion(empty_context: BlueskyContext): @@ -514,7 +514,7 @@ def default_movable(mov: Movable = inject("demo")) -> MsgGenerator: spec = empty_context._type_spec_for_function(default_movable) movable_ref = empty_context._reference(Movable) assert spec["mov"][0] == movable_ref - assert spec["mov"][1].default_factory == DefaultFactory("demo") + assert spec["mov"][1].default == "demo" def test_generic_default_device_reference(empty_context: BlueskyContext): @@ -524,7 +524,7 @@ def default_movable(mov: Movable[float] = inject("demo")) -> MsgGenerator: spec = empty_context._type_spec_for_function(default_movable) motor_ref = empty_context._reference(Movable[float]) assert spec["mov"][0] == motor_ref - assert spec["mov"][1].default_factory == DefaultFactory("demo") + assert spec["mov"][1].default == "demo" class ConcreteStoppable(Stoppable): @@ -574,7 +574,7 @@ def test_str_default(empty_context: BlueskyContext, sim_motor: Motor, alt_motor: spec = empty_context._type_spec_for_function(has_default_reference) assert spec["m"][0] is movable_ref - assert (df := spec["m"][1].default_factory) and df() == SIM_MOTOR_NAME # type: ignore + assert spec["m"][1].default == SIM_MOTOR_NAME assert has_default_reference.__name__ in empty_context.plans model = empty_context.plans[has_default_reference.__name__].model @@ -593,7 +593,7 @@ def test_nested_str_default( spec = empty_context._type_spec_for_function(has_default_nested_reference) assert spec["m"][0] == list[movable_ref] - assert (df := spec["m"][1].default_factory) and df() == [SIM_MOTOR_NAME] # type: ignore + assert spec["m"][1].default == [SIM_MOTOR_NAME] assert has_default_nested_reference.__name__ in empty_context.plans model = empty_context.plans[has_default_nested_reference.__name__].model @@ -697,7 +697,7 @@ def demo_plan(foo: int | None = None) -> MsgGenerator: empty_context.register_plan(demo_plan) schema = empty_context.plans["demo_plan"].model.model_json_schema() assert schema["properties"] == { - "foo": {"title": "Foo", "type": "integer"}, + "foo": {"title": "Foo", "type": "integer", "default": None}, } assert "foo" not in schema.get("required", []) @@ -725,7 +725,11 @@ def demo_plan(foo: int | str | None = None) -> MsgGenerator: empty_context.register_plan(demo_plan) schema = empty_context.plans["demo_plan"].model.model_json_schema() assert schema["properties"] == { - "foo": {"title": "Foo", "anyOf": [{"type": "integer"}, {"type": "string"}]} + "foo": { + "title": "Foo", + "anyOf": [{"type": "integer"}, {"type": "string"}], + "default": None, + } } assert "foo" not in schema.get("required", []) @@ -739,7 +743,10 @@ def demo_plan(foo: int | None) -> MsgGenerator: empty_context.register_plan(demo_plan) schema = empty_context.plans["demo_plan"].model.model_json_schema() assert schema["properties"] == { - "foo": {"title": "Foo", "anyOf": [{"type": "integer"}, {"type": "null"}]} + "foo": { + "title": "Foo", + "anyOf": [{"type": "integer"}, {"type": "null"}], + } } assert "foo" in schema.get("required", []) From 3ce0eff0c9836ae16aac98b6b50bed9b01549e75 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 7 Apr 2026 10:54:32 +0100 Subject: [PATCH 09/13] Fix system tests --- tests/system_tests/plans.json | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/system_tests/plans.json b/tests/system_tests/plans.json index 0124b86577..6ecd93ebae 100644 --- a/tests/system_tests/plans.json +++ b/tests/system_tests/plans.json @@ -20,7 +20,8 @@ }, "num": { "title": "Num", - "type": "integer" + "type": "integer", + "default": 1 }, "delay": { "anyOf": [ @@ -34,12 +35,14 @@ "type": "array" } ], + "default": 0.0, "title": "Delay" }, "metadata": { "additionalProperties": true, "title": "Metadata", - "type": "object" + "type": "object", + "default": "None" } }, "required": [ From bd6ceae4a70a06a9cec15e89a553d24c04548890 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 7 Apr 2026 11:10:57 +0100 Subject: [PATCH 10/13] Use null rather than "None" --- tests/system_tests/plans.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system_tests/plans.json b/tests/system_tests/plans.json index 6ecd93ebae..307e5073ab 100644 --- a/tests/system_tests/plans.json +++ b/tests/system_tests/plans.json @@ -42,7 +42,7 @@ "additionalProperties": true, "title": "Metadata", "type": "object", - "default": "None" + "default": null } }, "required": [ From afd9841e36b50c3bbf1715528e6471734a413a51 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 7 Apr 2026 13:28:04 +0100 Subject: [PATCH 11/13] Improve test to include optional parameter --- tests/unit_tests/core/test_context.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/unit_tests/core/test_context.py b/tests/unit_tests/core/test_context.py index c089ed9873..f659d72fd4 100644 --- a/tests/unit_tests/core/test_context.py +++ b/tests/unit_tests/core/test_context.py @@ -1,9 +1,10 @@ from __future__ import annotations from dataclasses import dataclass, field +from inspect import Parameter from pathlib import Path from types import ModuleType, NoneType -from typing import Any, Generic, TypeVar, Union +from typing import Any, Generic, TypeVar, Union, get_args, get_type_hints from unittest.mock import ANY, MagicMock, Mock, patch import pytest @@ -88,6 +89,10 @@ def has_some_params(foo: int = 42, bar: str = "bar") -> MsgGenerator: yield from () +def has_optional_parameter(foo: dict[str, Any] | None = None) -> MsgGenerator: + yield from () + + def has_typeless_param(foo) -> MsgGenerator: yield from () @@ -169,7 +174,9 @@ def some_configurable() -> SomeConfigurable: return SomeConfigurable() -@pytest.mark.parametrize("plan", [has_no_params, has_one_param, has_some_params]) +@pytest.mark.parametrize( + "plan", [has_no_params, has_one_param, has_some_params, has_optional_parameter] +) def test_add_plan(empty_context: BlueskyContext, plan: PlanGenerator): empty_context.register_plan(plan) assert plan.__name__ in empty_context.plans @@ -428,7 +435,7 @@ def test_with_config_passes_mock_to_with_dodal_module( mock_with_dodal_module.assert_called_once_with(ANY, mock=mock) -def test_function_spec(empty_context: BlueskyContext): +def test_function_spec_with_some_params(empty_context: BlueskyContext): spec = empty_context._type_spec_for_function(has_some_params) assert spec["foo"][0] is int assert spec["foo"][1].default == 42 @@ -436,6 +443,17 @@ def test_function_spec(empty_context: BlueskyContext): assert spec["bar"][1].default == "bar" +def test_function_spec_with_optional_params(empty_context: BlueskyContext): + spec = empty_context._type_spec_for_function(has_optional_parameter) + types = get_type_hints(has_optional_parameter) + arg_type = types.get("foo", Parameter.empty) + + _type = SkipJsonSchema[empty_context._convert_type(arg_type, False)] + inner_type, *annotations = get_args(_type) + assert spec["foo"][0] == inner_type + assert spec["foo"][1].default is None + + def test_basic_type_conversion(empty_context: BlueskyContext): assert empty_context._convert_type(int) is int assert empty_context._convert_type(dict[str, int]) == dict[str, int] From 0d0c85e0d8b6612b583a3c8a106f136628ab0354 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 7 Apr 2026 13:43:43 +0100 Subject: [PATCH 12/13] Add missing defaults --- tests/system_tests/plans.json | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/system_tests/plans.json b/tests/system_tests/plans.json index 307e5073ab..e55e3b474b 100644 --- a/tests/system_tests/plans.json +++ b/tests/system_tests/plans.json @@ -684,7 +684,8 @@ "metadata": { "additionalProperties": true, "title": "Metadata", - "type": "object" + "type": "object", + "default": null } }, "required": [ @@ -714,11 +715,13 @@ }, "group": { "title": "Group", - "type": "string" + "type": "string", + "default": null }, "wait": { "title": "Wait", - "type": "boolean" + "type": "boolean", + "default": false } }, "required": [ @@ -748,11 +751,13 @@ }, "group": { "title": "Group", - "type": "string" + "type": "string", + "default": null }, "wait": { "title": "Wait", - "type": "boolean" + "type": "boolean", + "default": false } }, "required": [ @@ -776,7 +781,8 @@ }, "group": { "title": "Group", - "type": "string" + "type": "string", + "default": null } }, "required": [ @@ -799,7 +805,8 @@ }, "group": { "title": "Group", - "type": "string" + "type": "string", + "default": null } }, "required": [ @@ -835,11 +842,13 @@ "properties": { "group": { "title": "Group", - "type": "string" + "type": "string", + "default": null }, "timeout": { "title": "Timeout", - "type": "number" + "type": "number", + "default": null } }, "title": "wait", From 951eb7f917c6d1377ce2a1562b9112becf642c24 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 7 Apr 2026 14:18:12 +0100 Subject: [PATCH 13/13] Add additional code coverage --- tests/unit_tests/client/test_client.py | 36 ++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index 5a7842fd9f..111aa0c8c6 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -748,6 +748,42 @@ def test_plan_multi_parameter_fallback_help_text(client): ) +def test_plan_help_text_with_ref(client): + schema = { + "$defs": { + "Spec": { + "properties": { + "foo": {"type": "integer"}, + "bar": {"$ref": "#/$defs/InnerSpec"}, + }, + "required": ["foo", "bar"], + }, + "InnerSpec": { + "properties": { + "x": {"type": "number"}, + "y": {"default": 10, "type": "number"}, + }, + "required": ["x"], + }, + }, + "properties": { + "spec": {"$ref": "#/$defs/Spec"}, + "meta": {"type": "string", "default": "abc"}, + }, + "required": ["spec"], + } + + plan = Plan( + "ref_plan", + PlanModel(name="ref_plan", schema=schema), + client, + ) + + expected = "Plan ref_plan(spec: Spec, meta: str = 'abc')" + + assert plan.help_text == expected + + def test_plan_properties(client): plan = Plan( "foo",