diff --git a/src/blueapi/client/device_cache.py b/src/blueapi/client/device_cache.py index db305e397..8c6841315 100644 --- a/src/blueapi/client/device_cache.py +++ b/src/blueapi/client/device_cache.py @@ -10,7 +10,7 @@ class DeviceCache: def __init__(self, rest: RestClientProtocol): self._rest = rest self._cache = { - model.name: DeviceRef(name=model.name, cache=self, model=model) + model.name: DeviceRef(cache=self, model=model) for model in rest.get_devices().devices } for name, device in self._cache.items(): @@ -37,7 +37,7 @@ def __getitem__(self, name: str) -> "DeviceRef": return dev try: model = self._rest.get_device(name) - device = DeviceRef(name=name, cache=self, model=model) + device = DeviceRef(cache=self, model=model) self._cache[name] = device setattr(self, model.name, device) return device @@ -63,19 +63,17 @@ class DeviceRef(ClientObjectRef): ``devices.my_device.child``. """ - name: str model: DeviceModel _cache: DeviceCache - def __init__(self, name: str, cache: DeviceCache, model: DeviceModel): - self.name = name + def __init__(self, cache: DeviceCache, model: DeviceModel): self.model = model self._cache = cache def __getattr__(self, name: str) -> "DeviceRef": if name.startswith("_"): raise AttributeError(f"No child device named {name}") - return self._cache[f"{self.name}.{name}"] + return self._cache[f"{self.model.name}.{name}"] def __repr__(self): - return f"Device({self.name})" + return f"Device({self.model.name})" diff --git a/src/blueapi/client/plan_cache.py b/src/blueapi/client/plan_cache.py index 69e64a1ab..127c71cbd 100644 --- a/src/blueapi/client/plan_cache.py +++ b/src/blueapi/client/plan_cache.py @@ -29,10 +29,7 @@ class PlanCache: """Collection of plans that can be accessed by name or attribute.""" def __init__(self, client: ClientProtocol, plans: list[PlanModel]): - self._cache = { - model.name: Plan(name=model.name, model=model, client=client) - for model in plans - } + self._cache = {model.name: Plan(model=model, client=client) for model in plans} for name, plan in self._cache.items(): if name.startswith("_"): continue @@ -54,8 +51,7 @@ def __repr__(self) -> str: class Plan: """Callable client-side reference to a registered BlueAPI plan.""" - def __init__(self, name: str, model: PlanModel, client: ClientProtocol): - self.name = name + def __init__(self, model: PlanModel, client: ClientProtocol): self.model = model self._client = client self.__doc__ = model.description @@ -75,7 +71,7 @@ def __call__(self, *args, **kwargs) -> Any: parameter schema. """ req = TaskRequest( - name=self.name, + name=self.model.name, params=self._build_args(*args, **kwargs), instrument_session=self._client.instrument_session, ) @@ -117,9 +113,9 @@ def _build_args(self, *args, **kwargs) -> dict[str, Any]: ) if len(args) > len(self.properties): - raise TypeError(f"{self.name} got too many arguments") + raise TypeError(f"{self.model.name} got too many arguments") if extra := {k for k in kwargs if k not in self.properties}: - raise TypeError(f"{self.name} got unexpected arguments: {extra}") + raise TypeError(f"{self.model.name} got unexpected arguments: {extra}") params = {} # Initially fill parameters using positional args assuming the order @@ -131,7 +127,7 @@ def _build_args(self, *args, **kwargs) -> dict[str, Any]: for key, value in kwargs.items(): # If we've already assumed a positional arg was this value, bail out if key in params: - raise TypeError(f"{self.name} got multiple values for {key}") + raise TypeError(f"{self.model.name} got multiple values for {key}") params[key] = value if missing := {k for k in self.required if k not in params}: @@ -158,7 +154,7 @@ def _format_arg(name: str, info: dict[str, Any]) -> str: return f"{name}: {typ} | None = None" args = [_format_arg(name, info) for name, info in self.properties.items()] - single_line = f"{self.name}({', '.join(args)})" + single_line = f"{self.model.name}({', '.join(args)})" if len(single_line) <= _REPR_MAX_LENGTH and len(args) <= _REPR_MAX_ARGS_INLINE: return single_line @@ -166,7 +162,7 @@ def _format_arg(name: str, info: dict[str, Any]) -> str: indent = " " # Fall back to multiline if too many arguments or too long. multiline_args = ",\n".join(f"{indent}{arg}" for arg in args) - return f"{self.name}(\n{multiline_args}\n)" + return f"{self.model.name}(\n{multiline_args}\n)" def _pretty_type(schema: dict[str, Any]) -> str: diff --git a/src/blueapi/client/protocols.py b/src/blueapi/client/protocols.py index dbb923af6..8093e657e 100644 --- a/src/blueapi/client/protocols.py +++ b/src/blueapi/client/protocols.py @@ -28,5 +28,4 @@ def get_device(self, name: str) -> DeviceModel: ... class ClientObjectRef: - name: str model: DeviceModel diff --git a/src/blueapi/client/rest.py b/src/blueapi/client/rest.py index 166620ac2..59dbec4f1 100644 --- a/src/blueapi/client/rest.py +++ b/src/blueapi/client/rest.py @@ -432,5 +432,5 @@ class ServiceUnavailableError(Exception): def _task_model_fallback(obj: Any) -> Any: """Fallback method for serializing TaskRequests""" if isinstance(obj, ClientObjectRef): - return obj.name + return obj.model.name raise PydanticSerializationError(f"Object of type {type(obj)} not serializable") diff --git a/tests/system_tests/test_blueapi_client.py b/tests/system_tests/test_blueapi_client.py index 540efeff4..fb3e4bebc 100644 --- a/tests/system_tests/test_blueapi_client.py +++ b/tests/system_tests/test_blueapi_client.py @@ -168,7 +168,7 @@ def test_missing_sub_device(bc: BlueapiClient): def test_plans_iterable(bc: BlueapiClient): - names = {plan.name for plan in bc.plans} + names = {plan.model.name for plan in bc.plans} assert {"count", "sleep", "set_absolute"} <= names assert all(isinstance(plan, Plan) for plan in bc.plans) @@ -307,7 +307,7 @@ def test_reload_environment_clears_caches(bc: BlueapiClient): assert new_env.environment_id != old_env.environment_id assert bc.plans is not plans assert bc.devices is not devices - assert {p.name for p in bc.plans} == {p.name for p in plans} + assert {p.model.name for p in bc.plans} == {p.model.name for p in plans} def test_callbacks_receive_worker_events(bc: BlueapiClient): diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index e7e31f930..e9b942300 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -505,7 +505,6 @@ def test_scripting_interface_returns_result(): result=TaskResult(result=42, type="int"), ) demo_plan = Plan( - name="demo", client=client, model=PlanModel(name="demo", description="Demo plan", schema={}), ) @@ -521,7 +520,6 @@ def test_scripting_interface_raises_exceptions(): result=TaskError(type="ValueError", message="Plan failed"), ) demo_plan = Plan( - name="demo", client=client, model=PlanModel(name="demo", description="Demo plan", schema={}), ) diff --git a/tests/unit_tests/client/test_device_cache.py b/tests/unit_tests/client/test_device_cache.py index 161e7d0ff..cb1f75273 100644 --- a/tests/unit_tests/client/test_device_cache.py +++ b/tests/unit_tests/client/test_device_cache.py @@ -39,7 +39,7 @@ def test_device_repr(): cache = Mock() model = Mock() model.name = "foo" - dev = DeviceRef(name="foo", cache=cache, model=model) + dev = DeviceRef(cache=cache, model=model) assert repr(dev) == "Device(foo)" @@ -47,7 +47,7 @@ def test_device_ignores_underscores(): cache = MagicMock() model = Mock() model.name = "foo" - dev = DeviceRef(name="foo", cache=cache, model=model) + dev = DeviceRef(cache=cache, model=model) with pytest.raises(AttributeError, match="_underscore"): _ = dev._underscore cache.__getitem__.assert_not_called() diff --git a/tests/unit_tests/client/test_plan_cache.py b/tests/unit_tests/client/test_plan_cache.py index 3da4d1c9b..4ee5105ab 100644 --- a/tests/unit_tests/client/test_plan_cache.py +++ b/tests/unit_tests/client/test_plan_cache.py @@ -33,13 +33,12 @@ def test_plan_cache_repr(client): def test_plan_help_text(client): - plan = Plan("foo", PlanModel(name="foo", description="help for foo"), client) + plan = Plan(PlanModel(name="foo", description="help for foo"), client) assert plan.help_text == "help for foo" def test_plan_fallback_help_text(client): plan = Plan( - "foo", PlanModel( name="foo", schema={"properties": {"one": {}, "two": {}}, "required": ["one"]}, @@ -51,7 +50,6 @@ def test_plan_fallback_help_text(client): def test_plan_multi_parameter_fallback_help_text(client): plan = Plan( - "foo", PlanModel( name="foo", schema={ @@ -102,7 +100,7 @@ def test_plan_help_text_with_ref(client): "required": ["spec"], } - plan = Plan("ref_plan", PlanModel(name="ref_plan", schema=schema), client) + plan = Plan(PlanModel(name="ref_plan", schema=schema), client) expected = "Plan ref_plan(spec: Spec, meta: str = 'abc')" assert plan.help_text == expected @@ -110,7 +108,6 @@ def test_plan_help_text_with_ref(client): def test_plan_properties(client): plan = Plan( - "foo", PlanModel( name="foo", schema={"properties": {"one": {}, "two": {}}, "required": ["one"]}, @@ -123,7 +120,7 @@ def test_plan_properties(client): def test_plan_empty_fallback_help_text(client): plan = Plan( - "foo", PlanModel(name="foo", schema={"properties": {}, "required": []}), client + PlanModel(name="foo", schema={"properties": {}, "required": []}), client ) assert plan.help_text == "Plan foo()" @@ -144,7 +141,7 @@ def test_plan_empty_fallback_help_text(client): def test_plan_param_mapping(args, kwargs, params): client = Mock() client.instrument_session = "cm12345-1" - plan = Plan("foobar", FULL_PLAN, client) + plan = Plan(FULL_PLAN, client) plan(*args, **kwargs) client.run_task.assert_called_once_with( @@ -170,7 +167,7 @@ def test_plan_param_mapping(args, kwargs, params): def test_plan_invalid_param_mapping(args, kwargs, msg): client = Mock() client.instrument_session = "cm12345-1" - plan = Plan("foobar", FULL_PLAN, client) + plan = Plan(FULL_PLAN, client) with pytest.raises(TypeError, match=msg): plan(*args, **kwargs) diff --git a/tests/unit_tests/client/test_rest.py b/tests/unit_tests/client/test_rest.py index 8c8c004cd..a558e518f 100644 --- a/tests/unit_tests/client/test_rest.py +++ b/tests/unit_tests/client/test_rest.py @@ -96,7 +96,7 @@ def test_create_task_serialization(): request = TaskRequest( name="demo", instrument_session="cm12345-1", - params={"devices": [DeviceRef(name="foo", cache=Mock(), model=model)]}, + params={"devices": [DeviceRef(cache=Mock(), model=model)]}, ) BlueapiRestClient.create_task(rest, request)