From a1ad5cddd3af04921959e24cdea9811b442187d7 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 13 Feb 2026 12:41:07 +0000 Subject: [PATCH 1/4] feat: validate methods and extract helper functions --- src/fastcs/controllers/base_controller.py | 140 ++++++++++++++-------- src/fastcs/methods/__init__.py | 1 + 2 files changed, 93 insertions(+), 48 deletions(-) diff --git a/src/fastcs/controllers/base_controller.py b/src/fastcs/controllers/base_controller.py index 0e5fc1de..71a2cdb4 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -7,7 +7,7 @@ from fastcs.attributes import AnyAttributeIO, Attribute, AttrR, AttrW, HintedAttribute from fastcs.logging import bind_logger -from fastcs.methods import Command, Scan, UnboundCommand, UnboundScan +from fastcs.methods import Command, Method, Scan, UnboundCommand, UnboundScan from fastcs.tracer import Tracer logger = bind_logger(logger_name=__name__) @@ -52,6 +52,7 @@ def __init__( self.__hinted_attributes: dict[str, HintedAttribute] = {} self.__hinted_sub_controllers: dict[str, type[BaseController]] = {} + self.__hinted_methods: dict[str, type[Method]] = {} self._find_type_hints() self._bind_attrs() @@ -87,6 +88,9 @@ def _find_type_hints(self): elif isinstance(hint, type) and issubclass(hint, BaseController): self.__hinted_sub_controllers[name] = hint + elif isinstance(hint, type) and issubclass(hint, Method): + self.__hinted_methods[name] = hint + def _bind_attrs(self) -> None: """Search for Attributes and Methods to bind them to this instance. @@ -168,47 +172,77 @@ def post_initialise(self): self._connect_attribute_ios() def _validate_type_hints(self): - """Validate all `Attribute` and `Controller` type-hints were introspected""" + """Validate all `Attribute`, `Controller`, and `Method` + type-hints were introspected""" for name in self.__hinted_attributes: self._validate_hinted_attribute(name) for name in self.__hinted_sub_controllers: self._validate_hinted_controller(name) + for name in self.__hinted_methods: + self._validate_hinted_method(name) + for subcontroller in self.sub_controllers.values(): subcontroller._validate_type_hints() # noqa: SLF001 + def _validate_hinted_member(self, name: str, expected_type: type): + """Validate thata hinted member exists on the controller""" + member = getattr(self, name, None) + if member is None or not isinstance(member, expected_type): + raise RuntimeError() + return member + + def _validate_hinted_method(self, name: str): + """Check that a `Method` with the given name exists on the controller""" + try: + method = self._validate_hinted_member(name, Method) + except RuntimeError as exc: + raise RuntimeError( + f"Controller `{self.__class__.__name__}` failed to introspect " + f"hinted method `{name}` during initialisation" + ) from exc + + logger.debug( + "Validated hinted method", + name=name, + controller=self, + method=method, + ) + def _validate_hinted_attribute(self, name: str): """Check that an `Attribute` with the given name exists on the controller""" - attr = getattr(self, name, None) - if attr is None or not isinstance(attr, Attribute): + try: + attr = self._validate_hinted_member(name, Attribute) + except RuntimeError as exc: raise RuntimeError( f"Controller `{self.__class__.__name__}` failed to introspect " f"hinted attribute `{name}` during initialisation" - ) - else: - logger.debug( - "Validated hinted attribute", - name=name, - controller=self, - attribute=attr, - ) + ) from exc + + logger.debug( + "Validated hinted attribute", + name=name, + controller=self, + attribute=attr, + ) def _validate_hinted_controller(self, name: str): """Check that a sub controller with the given name exists on the controller""" - controller = getattr(self, name, None) - if controller is None or not isinstance(controller, BaseController): + try: + controller = self._validate_hinted_member(name, BaseController) + except RuntimeError as exc: raise RuntimeError( f"Controller `{self.__class__.__name__}` failed to introspect " f"hinted controller `{name}` during initialisation" - ) - else: - logger.debug( - "Validated hinted sub controller", - name=name, - controller=self, - sub_controller=controller, - ) + ) from exc + + logger.debug( + "Validated hinted sub controller", + name=name, + controller=self, + sub_controller=controller, + ) def _connect_attribute_ios(self) -> None: """Connect ``Attribute`` callbacks to ``AttributeIO``s""" @@ -245,14 +279,27 @@ def set_path(self, path: list[str]): for attribute in self.__attributes.values(): attribute.set_path(path) + def _check_for_name_clash(self, name: str): + namespaces = { + "attribute": self.__attributes, + "sub controller": self.__sub_controllers, + "scan method": self.__scan_methods, + "command method": self.__command_methods, + } + + for kind, namespace in namespaces.items(): + if name in namespace: + raise ValueError( + f"Controller {self} has existing {kind} {name}: {namespace[name]}" + ) + def add_attribute(self, name, attr: Attribute): - if name in self.__attributes: - raise ValueError( - f"Cannot add attribute {attr}. " - f"Controller {self} has has existing attribute {name}: " - f"{self.__attributes[name]}" - ) - elif name in self.__hinted_attributes: + try: + self._check_for_name_clash(name) + except ValueError as exc: + raise ValueError(f"Cannot add attribute {attr}.") from exc + + if name in self.__hinted_attributes: hint = self.__hinted_attributes[name] if not isinstance(attr, hint.attr_type): raise RuntimeError( @@ -267,12 +314,6 @@ def add_attribute(self, name, attr: Attribute): f"Expected '{hint.dtype.__name__}', " f"got '{attr.datatype.dtype.__name__}'." ) - elif name in self.__sub_controllers.keys(): - raise ValueError( - f"Cannot add attribute {attr}. " - f"Controller {self} has existing sub controller {name}: " - f"{self.__sub_controllers[name]}" - ) attr.set_name(name) attr.set_path(self.path) @@ -284,13 +325,12 @@ def attributes(self) -> dict[str, Attribute]: return self.__attributes def add_sub_controller(self, name: str, sub_controller: BaseController): - if name in self.__sub_controllers.keys(): - raise ValueError( - f"Cannot add sub controller {sub_controller}. " - f"Controller {self} has existing sub controller {name}: " - f"{self.__sub_controllers[name]}" - ) - elif name in self.__hinted_sub_controllers: + try: + self._check_for_name_clash(name) + except ValueError as exc: + raise ValueError(f"Cannot add sub controller {sub_controller}.") from exc + + if name in self.__hinted_sub_controllers: hint = self.__hinted_sub_controllers[name] if not isinstance(sub_controller, hint): raise RuntimeError( @@ -299,12 +339,6 @@ def add_sub_controller(self, name: str, sub_controller: BaseController): f"Expected '{hint.__name__}' got " f"'{sub_controller.__class__.__name__}'." ) - elif name in self.__attributes: - raise ValueError( - f"Cannot add sub controller {sub_controller}. " - f"Controller {self} has existing attribute {name}: " - f"{self.__attributes[name]}" - ) sub_controller.set_path(self.path + [name]) self.__sub_controllers[name] = sub_controller @@ -318,6 +352,11 @@ def sub_controllers(self) -> dict[str, BaseController]: return self.__sub_controllers def add_command(self, name: str, command: Command): + try: + self._check_for_name_clash(name) + except ValueError as exc: + raise ValueError(f"Cannot add command method {command}.") from exc + self.__command_methods[name] = command super().__setattr__(name, command) @@ -326,6 +365,11 @@ def command_methods(self) -> dict[str, Command]: return self.__command_methods def add_scan(self, name: str, scan: Scan): + try: + self._check_for_name_clash(name) + except ValueError as exc: + raise ValueError(f"Cannot add scan method {scan}.") from exc + self.__scan_methods[name] = scan super().__setattr__(name, scan) diff --git a/src/fastcs/methods/__init__.py b/src/fastcs/methods/__init__.py index abe6d61f..0cdeb616 100644 --- a/src/fastcs/methods/__init__.py +++ b/src/fastcs/methods/__init__.py @@ -2,6 +2,7 @@ from .command import CommandCallback as CommandCallback from .command import UnboundCommand as UnboundCommand from .command import command as command +from .method import Method as Method from .scan import Scan as Scan from .scan import ScanCallback as ScanCallback from .scan import UnboundScan as UnboundScan From 5ce671d6d493d51926c68152c4c900c9944d4a5d Mon Sep 17 00:00:00 2001 From: root Date: Fri, 13 Feb 2026 12:41:40 +0000 Subject: [PATCH 2/4] tests: amend regex in tests --- tests/test_controllers.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tests/test_controllers.py b/tests/test_controllers.py index bc084324..69cd4c82 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -20,7 +20,7 @@ def test_controller_nesting(): assert controller.sub_controllers == {"a": sub_controller} assert sub_controller.sub_controllers == {"b": sub_sub_controller} - with pytest.raises(ValueError, match=r"existing sub controller"): + with pytest.raises(ValueError, match=r"Cannot add sub controller"): controller.a = Controller() with pytest.raises(ValueError, match=r"already registered"): @@ -86,22 +86,16 @@ def __init__(self): controller = ConflictingController() - with pytest.raises(ValueError, match=r"Cannot add attribute .* existing attribute"): + with pytest.raises(ValueError, match=r"Cannot add attribute"): controller.attr = AttrR(Float()) # pyright: ignore[reportAttributeAccessIssue] - with pytest.raises( - ValueError, match=r"Cannot add sub controller .* existing attribute" - ): + with pytest.raises(ValueError, match=r"Cannot add sub controller"): controller.attr = Controller() # pyright: ignore[reportAttributeAccessIssue] - with pytest.raises( - ValueError, match=r"Cannot add sub controller .* existing sub controller" - ): + with pytest.raises(ValueError, match=r"Cannot add sub controller"): controller.sub_controller = Controller() - with pytest.raises( - ValueError, match=r"Cannot add attribute .* existing sub controller" - ): + with pytest.raises(ValueError, match=r"Cannot add attribute"): controller.sub_controller = AttrR(Int()) # pyright: ignore[reportAttributeAccessIssue] From b2c003d264b4705cf609cc88b7ff24638e680650 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 13 Feb 2026 13:09:14 +0000 Subject: [PATCH 3/4] test: add tests for method validation --- src/fastcs/controllers/base_controller.py | 21 +++++++-- tests/test_controllers.py | 55 ++++++++++++++++++----- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/fastcs/controllers/base_controller.py b/src/fastcs/controllers/base_controller.py index 71a2cdb4..e21ac798 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -351,11 +351,23 @@ def add_sub_controller(self, name: str, sub_controller: BaseController): def sub_controllers(self) -> dict[str, BaseController]: return self.__sub_controllers + def _validated_added_method(self, name: str, method: Method): + if name in self.__hinted_methods: + hint = self.__hinted_methods[name] + if not isinstance(method, hint): + raise RuntimeError( + f"Controller '{self.__class__.__name__}' introspection of " + f"hinted method '{name}' does not match defined type. " + f"Expected '{hint.__name__}' got " + f"'{method.__class__.__name__}'." + ) + def add_command(self, name: str, command: Command): try: self._check_for_name_clash(name) - except ValueError as exc: - raise ValueError(f"Cannot add command method {command}.") from exc + self._validated_added_method(name, command) + except (ValueError, RuntimeError) as exc: + raise exc.__class__(f"Cannot add command method {command}.") from exc self.__command_methods[name] = command super().__setattr__(name, command) @@ -367,8 +379,9 @@ def command_methods(self) -> dict[str, Command]: def add_scan(self, name: str, scan: Scan): try: self._check_for_name_clash(name) - except ValueError as exc: - raise ValueError(f"Cannot add scan method {scan}.") from exc + self._validated_added_method(name, scan) + except (ValueError, RuntimeError) as exc: + raise exc.__class__(f"Cannot add command method {scan}.") from exc self.__scan_methods[name] = scan super().__setattr__(name, scan) diff --git a/tests/test_controllers.py b/tests/test_controllers.py index 69cd4c82..25b194c7 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -5,6 +5,7 @@ from fastcs.attributes import AttrR, AttrRW from fastcs.controllers import Controller, ControllerVector from fastcs.datatypes import Enum, Float, Int +from fastcs.methods import Command, Scan def test_controller_nesting(): @@ -76,9 +77,30 @@ def test_attribute_parsing(): } -def test_conflicting_attributes_and_controllers(): +async def noop() -> None: + pass + + +@pytest.mark.parametrize( + "member_name, member_value, expected_error", + [ + ("attr", AttrR(Float()), r"Cannot add attribute"), + ("attr", Controller(), r"Cannot add sub controller"), + ("attr", Command(noop), r"Cannot add command"), + ("sub_controller", AttrR(Int()), r"Cannot add attribute"), + ("sub_controller", Controller(), r"Cannot add sub controller"), + ("sub_controller", Command(noop), r"Cannot add command"), + ("cmd", AttrR(Int()), r"Cannot add attribute"), + ("cmd", Controller(), r"Cannot add sub controller"), + ("cmd", Command(noop), r"Cannot add command"), + ], +) +def test_conflicting_attributes_and_controllers_and_commands( + member_name, member_value, expected_error +): class ConflictingController(Controller): attr = AttrR(Int()) + cmd = Command(noop) def __init__(self): super().__init__() @@ -86,17 +108,8 @@ def __init__(self): controller = ConflictingController() - with pytest.raises(ValueError, match=r"Cannot add attribute"): - controller.attr = AttrR(Float()) # pyright: ignore[reportAttributeAccessIssue] - - with pytest.raises(ValueError, match=r"Cannot add sub controller"): - controller.attr = Controller() # pyright: ignore[reportAttributeAccessIssue] - - with pytest.raises(ValueError, match=r"Cannot add sub controller"): - controller.sub_controller = Controller() - - with pytest.raises(ValueError, match=r"Cannot add attribute"): - controller.sub_controller = AttrR(Int()) # pyright: ignore[reportAttributeAccessIssue] + with pytest.raises(ValueError, match=expected_error): + setattr(controller, member_name, member_value) def test_controller_raises_error_if_passed_numeric_sub_controller_name(): @@ -197,3 +210,21 @@ class HintedController(Controller): controller.add_sub_controller("child", SomeSubController()) controller._validate_type_hints() + + +@pytest.mark.asyncio +async def test_method_hint_validation(): + class HintedController(Controller): + method: Scan + + controller = HintedController() + + with pytest.raises(RuntimeError, match="failed to introspect hinted method"): + controller._validate_type_hints() + + with pytest.raises(RuntimeError, match="Cannot add command method"): + controller.add_command("method", Command(noop)) + + controller.add_scan("method", Scan(fn=noop, period=0.1)) + + controller._validate_type_hints() From ab377030d0c5f50fa991ae23d2d19c0c61216ae9 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 13 Feb 2026 13:11:25 +0000 Subject: [PATCH 4/4] chore: amend typo --- src/fastcs/controllers/base_controller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fastcs/controllers/base_controller.py b/src/fastcs/controllers/base_controller.py index e21ac798..37011cc4 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -187,7 +187,7 @@ def _validate_type_hints(self): subcontroller._validate_type_hints() # noqa: SLF001 def _validate_hinted_member(self, name: str, expected_type: type): - """Validate thata hinted member exists on the controller""" + """Validate that a hinted member exists on the controller""" member = getattr(self, name, None) if member is None or not isinstance(member, expected_type): raise RuntimeError() @@ -381,7 +381,7 @@ def add_scan(self, name: str, scan: Scan): self._check_for_name_clash(name) self._validated_added_method(name, scan) except (ValueError, RuntimeError) as exc: - raise exc.__class__(f"Cannot add command method {scan}.") from exc + raise exc.__class__(f"Cannot add scan method {scan}.") from exc self.__scan_methods[name] = scan super().__setattr__(name, scan)