diff --git a/claude.md b/claude.md index 65c11785b..034cb5265 100644 --- a/claude.md +++ b/claude.md @@ -53,6 +53,51 @@ Conventions for this repository. Follow these when writing or reviewing code and Rule of thumb: a helper's parameters should describe *what it's being asked to validate/produce*, never *whether/how the caller wants it validated*. +- The same applies to a parameter that only names *where the data came from* so the + helper can put it in its message (`source`, `origin`, `context`). The call-site + already knows that; if it wants it in the error, it should catch the helper's plain + exception and say so itself. Prefer a generic exception from the helper to a + parameter that exists only to decorate one. + + ```python + # Bad: `source` is threaded in purely to phrase the message + def check_filled(self, source: str | None = None) -> None: + ... + raise RuntimeError(f"did not provision from {source}: {missing}") + + # Good: the helper says what it knows + def check_filled(self) -> None: + ... + raise RuntimeError(f"did not provision: {missing}") + ``` + +## Typing + +- Do **not** annotate a parameter `Any` when a narrower type says what the function + actually accepts. `Any` in a signature is a promise the function cannot keep: it + turns an author-time error into a runtime one, or into no error at all. + + ```python + # Bad: any object at all type checks + def fill_attribute(self, name: str, datatype: Any = None) -> Attribute: ... + + # Good: only the types an attribute can hold + def fill_attribute(self, name: str, datatype: type[DType_T] | None = None) -> Attribute: ... + ``` + +- This holds especially for callables that will be attached to something already + typed. IO handed to an existing `Attribute` must be typed with the datatype + TypeVar (`Getter[DType_T]`, `Setter[DType_T]`), not `Getter[Any]` - otherwise a + setter taking a `datetime.datetime` type checks against a `float` attribute and only + fails, if at all, at runtime. Type it so the author is warned where they wrote it. + +- A `# type: ignore` or `# pyright: ignore` must be **required**. Before adding one, + remove it and re-run the type checker: if nothing is reported, it does not belong. + Before reaching for one at all, look for a signature that makes it unnecessary - + `**meta: Unpack[Meta]` rather than `**meta: Any` plus an ignore at the call that + passes them on. Every remaining ignore should be specific (`[reportCallIssue]`, not + bare) and should have a reason a reader can check. + ## Tests: `pytest.raises` - The `with pytest.raises(...):` block should contain the **minimal code that raises @@ -115,3 +160,84 @@ Conventions for this repository. Follow these when writing or reviewing code and This keeps tests readable as documentation: every line is evidence for the claim in the test's name, not incidental noise carried over from copy-pasting another test. + +## Tests: one behaviour per test + +- A test checks **one** thing, and its name says which. Do not put two `pytest.raises` + blocks, or a failing case and a happy path, in one test: they are separate claims + about the code, and when the first fails the rest never run. +- Setup shared by the split tests belongs in a fixture, a module-level class, or a + couple of repeated lines - repeating three lines of setup is cheaper than a test + that verifies four unrelated things. + + ```python + # Bad: three claims, one name, and the last two only run if the first passes + def test_hint_validation(): + controller = HintedController() + + with pytest.raises(RuntimeError, match="never added"): + controller.check_filled() + + with pytest.raises(RuntimeError, match="expected 'AttrR'"): + controller.add_attribute("state", AttrW(int)) + + controller.add_attribute("state", AttrR(int)) + controller.check_filled() + + # Good: one claim each, each named for what it checks + def test_a_hint_without_a_datatype_is_promised(): ... + def test_adding_a_promised_attribute_with_the_wrong_access_mode_raises(): ... + def test_adding_a_promised_attribute_satisfies_the_declaration(): ... + ``` + +## Tests: parametrize instead of near-identical tests + +- If two or more tests differ only in the values they use - a different hint, a + different datatype, a different expected message - write one + `@pytest.mark.parametrize`d test rather than copies of one body. +- Keep them separate when the *shape* of the test differs, not just its data: a case + that needs different setup, different assertions, or a different name to make sense + is a different test. + +## Unused names + +- Declare a name only where something reads it. An unused local, argument or + `@pytest.mark.parametrize` column is a reader's question with no answer: they have to + scan the whole body to find out that nothing uses it. +- This bites hardest when a parametrized test is split off another one. The columns are + copied across whole, and a column the new body never reads survives in every case: + + ```python + # Bad: `name` is in every case and read by nothing + @pytest.mark.parametrize( + "parent_type, name, expected", + [ + (ChildHintedParent, "child", "child .declared Child, never added."), + (VectorHintedParent, "children", "children .declared ControllerVector"), + ], + ) + def test_a_controller_hint_is_not_created( + parent_type: type[Controller], name: str, expected: str + ): + controller = parent_type() + + with pytest.raises(RuntimeError, match=expected): + controller.check_filled() + + # Good + @pytest.mark.parametrize( + "parent_type, expected", + [ + (ChildHintedParent, "child .declared Child, never added."), + (VectorHintedParent, "children .declared ControllerVector"), + ], + ) + def test_a_controller_hint_is_not_created( + parent_type: type[Controller], expected: str + ): + ... + ``` + +- The exception is a name a caller outside your control decides: a signature that + implements an interface, or an unpacking that has to consume every element. Prefix + those with an underscore rather than deleting them. diff --git a/docs/explanations/controllers.md b/docs/explanations/controllers.md index 30a09da0a..b697fca1a 100644 --- a/docs/explanations/controllers.md +++ b/docs/explanations/controllers.md @@ -20,11 +20,16 @@ lifecycle, if required. | Method | Purpose | |---|---| -| `initialise` | Dynamically add attributes on startup, before the API is built | +| `initialise` | Fill declared attributes, and add dynamic ones, before the API is built | | `connect` | Open connection to device | | `reconnect` | Re-open connection after scan error | | `disconnect` | Release device resources before shutdown | +Attributes are constructed in `__init__`, or declared as class-body type hints +and created for you - see [](declaring-attributes.md) for which to use when. +An `Attribute` assigned in the class body is rejected: one object would be +shared by every instance of the controller. + ### Scan task behaviour When used as the root controller, FastCS collects all `@scan` methods and readable @@ -41,8 +46,12 @@ from fastcs.methods import scan class TemperatureController(Controller): - temperature = AttrR(float, units="degC") - setpoint = AttrRW(float, units="degC") + def __init__(self, host, port): + super().__init__() + self._host, self._port = host, port + + self.temperature = AttrR(float, units="degC") + self.setpoint = AttrRW(float, units="degC") async def connect(self): self._client = await DeviceClient.connect(self._host, self._port) @@ -72,7 +81,9 @@ controller also has connection logic, the parent must invoke it explicitly: ```python class ChannelController(Controller): - value = AttrR(float) + def __init__(self): + super().__init__() + self.value = AttrR(float) async def connect(self): ... @@ -107,7 +118,9 @@ from fastcs.controllers import Controller, ControllerVector class ChannelController(Controller): - value = AttrR(float) + def __init__(self): + super().__init__() + self.value = AttrR(float) class RootController(Controller): diff --git a/docs/explanations/declaring-attributes.md b/docs/explanations/declaring-attributes.md new file mode 100644 index 000000000..f99535f97 --- /dev/null +++ b/docs/explanations/declaring-attributes.md @@ -0,0 +1,121 @@ +# Declaring Attributes: Class Body vs `__init__` + +A `Controller`'s class body holds **declarations**; its `__init__` holds +**construction with data**. Which of the two an attribute belongs in follows +from one question: is everything about the attribute known when you write the +class? + +## Construct it in `__init__` when you know everything + +If you can write the attribute down in full — its datatype, its metadata, and +the IO that reads and writes it — construct it in `__init__` and assign it to +`self`: + +```python +class PowerSupplyController(Controller): + def __init__(self, protocol: PowerSupplyProtocol) -> None: + super().__init__() + + self.voltage = AttrRW( + float, + getter=Polled(protocol.get_voltage, period=0.5), + setter=protocol.set_voltage, + units="V", + precision=3, + ) +``` + +This is most drivers, most of the time. Because the attribute is built per +instance, it can close over per-instance state — which is what lets one +channel's index be baked into its own getter rather than dispatched on at IO +time. + +An `Attribute` **may not** be assigned in the class body. One built there would +be a single object shared by every instance of the controller, so two devices +of the same model would write into each other; FastCS raises at construction +rather than let that happen, and names the attribute. + +## Declare it as a hint when the data arrives later + +Some drivers cannot write the attribute down in full, because what it needs +comes from the device — a detector that reports its own parameter tree — or +from a protocol library that turns one line of metadata into a getter and a +setter. Those declare a **type hint**, and `ControllerFiller` creates the +attribute from it: + +```python +class OdinDetector(Controller): + frames: AttrRW[int] + + async def initialise(self) -> None: + for name, spec in await self._query_parameter_tree(): + self.filler.fill_attribute( + name, getter=spec.getter, setter=spec.setter, **spec.meta + ) + + self.filler.check_filled() +``` + +The hint is not a promise to build something later. `self.frames` **exists as +soon as `__init__` returns** — as an `AttrRW[int]` with no IO yet — so the rest +of `__init__` can reference it, hand it to a sibling, or subscribe to it. That +rule is what makes `initialise` safe to run in parallel across controllers: +only `__init__` is serial, and by the time it ends every attribute anything +refers to is there. + +`fill_attribute` provisions the attribute **in place**, so a reference taken +during `__init__` is the same object that ends up serving the device. It +validates as it goes: the metadata against the datatype the hint declared +(`precision` on a `str` raises, naming the field and the attribute), and, when +you pass `datatype=`, what the device reported against what you declared. + +### A hint that cannot name its datatype + +Occasionally the datatype itself is only knowable over the wire — an enum whose +members the device reports. Write the hint without a subscript: + +```python +class EigerDetector(Controller): + state: AttrR # enum built from the device's `allowed_values` +``` + +FastCS cannot create that one, so it is a **promise** instead: introspection +must add it with `add_attribute`, and `check_filled` fails if nothing did. The +access mode is still checked — adding an `AttrW` where an `AttrR` was promised +raises. + +### Extras: metadata a protocol layer defines + +An `Annotated` hint carries anything else you put in it, and the filler hands +it back untouched: + +```python +class Instrument(SCPIController): + power: Annotated[AttrRW[float], SCPIParam("P", precision=3, units="W")] +``` + +Core FastCS defines **no** extras vocabulary. `SCPIParam` above belongs to +whatever protocol package you build on top: it reads the extras off each +declaration, builds the getter and setter its protocol implies, and fills the +attribute. This is how a protocol library gets a declarative spelling of its +own without FastCS knowing anything about it. + +## Checking what was promised + +`check_filled()` raises if anything the class body declared is missing, listing +it by name. FastCS calls it across the whole controller tree after +`initialise`, so a driver that forgets cannot serve a half-built API; call it +yourself at the end of your own `initialise` to fail before anything else runs. +An `| None` hint is not required. + +## Summary + +| You know | Write | +|---|---| +| Everything about the attribute | `self.x = AttrRW(...)` in `__init__` | +| Its type, but not its IO or metadata | `x: AttrRW[int]` and fill it in `initialise` | +| Its access mode only | `x: AttrR` and `add_attribute` it in `initialise` | +| Nothing until the device answers | No declaration; `add_attribute` in `initialise` | + +See [ADR 0013](decisions/0013-declarative-procedural-split-and-controller-filler.md) +for why there is one declarative mechanism rather than two. diff --git a/docs/how-to/arrange-epics-screens.md b/docs/how-to/arrange-epics-screens.md index 10b9c1649..1df1a5c46 100644 --- a/docs/how-to/arrange-epics-screens.md +++ b/docs/how-to/arrange-epics-screens.md @@ -20,12 +20,15 @@ from fastcs.methods import command class PowerSupplyController(Controller): - voltage = AttrRW(float, group="Output") - current = AttrRW(float, group="Output") - power = AttrR(float, group="Output") + def __init__(self) -> None: + super().__init__() + + self.voltage = AttrRW(float, group="Output") + self.current = AttrRW(float, group="Output") + self.power = AttrR(float, group="Output") - temperature = AttrR(float, group="Status") - fault_code = AttrR(int, group="Status") + self.temperature = AttrR(float, group="Status") + self.fault_code = AttrR(int, group="Status") @command(group="Actions") async def reset_faults(self) -> None: @@ -54,9 +57,12 @@ from fastcs.methods import command class ChannelController(Controller): - voltage = AttrRW(float, group="Output") - current = AttrRW(float, group="Output") - temperature = AttrR(float, group="Status") + def __init__(self) -> None: + super().__init__() + + self.voltage = AttrRW(float, group="Output") + self.current = AttrRW(float, group="Output") + self.temperature = AttrR(float, group="Status") @command(group="Actions") async def enable(self) -> None: @@ -64,14 +70,14 @@ class ChannelController(Controller): class MultiChannelPSU(Controller): - total_power = AttrR(float) - @command() async def disable_all(self) -> None: ... def __init__(self, num_channels: int) -> None: super().__init__() + + self.total_power = AttrR(float) for i in range(1, num_channels + 1): self.add_sub_controller(f"Ch{i:02d}", ChannelController()) ``` diff --git a/docs/how-to/table-waveform-data.md b/docs/how-to/table-waveform-data.md index a9cb4667d..51bba40e4 100644 --- a/docs/how-to/table-waveform-data.md +++ b/docs/how-to/table-waveform-data.md @@ -14,11 +14,14 @@ from fastcs.controllers import Controller from fastcs.datatypes import Array1D class SpectrumController(Controller): - # 1D array of 1000 float64 values - spectrum = AttrR(Array1D[np.float64], shape=(1000,)) + def __init__(self): + super().__init__() - # Writable array - setpoints = AttrRW(Array1D[np.float64], shape=(100,)) + # 1D array of 1000 float64 values + self.spectrum = AttrR(Array1D[np.float64], shape=(1000,)) + + # Writable array + self.setpoints = AttrRW(Array1D[np.float64], shape=(100,)) ``` ### 2D Arrays (Images) @@ -29,11 +32,14 @@ ophyd-async-compatible spelling, so write it as `np.ndarray` with an explicit ```python class CameraController(Controller): - # 2D array for images (max 1024x1024 uint16) - image = AttrR(np.ndarray, array_dtype=np.uint16, shape=(1024, 1024)) + def __init__(self): + super().__init__() + + # 2D array for images (max 1024x1024 uint16) + self.image = AttrR(np.ndarray, array_dtype=np.uint16, shape=(1024, 1024)) - # Smaller region of interest - roi = AttrRW(np.ndarray, array_dtype=np.uint16, shape=(256, 256)) + # Smaller region of interest + self.roi = AttrRW(np.ndarray, array_dtype=np.uint16, shape=(256, 256)) ``` ### Array Metadata @@ -49,7 +55,9 @@ class CameraController(Controller): from fastcs.methods import scan class SpectrumController(Controller): - spectrum = AttrR(Array1D[np.float64], shape=(1000,)) + def __init__(self): + super().__init__() + self.spectrum = AttrR(Array1D[np.float64], shape=(1000,)) @scan(period=0.1) async def read_spectrum(self): @@ -88,15 +96,18 @@ from fastcs.controllers import Controller from fastcs.datatypes import Table class MeasurementController(Controller): - # Table with columns: name (string), value (float), valid (bool) - results = AttrR( - Table, - structured_dtype=[ - ("name", "S32"), # 32-character string - ("value", np.float64), - ("valid", np.bool_), - ], - ) + def __init__(self): + super().__init__() + + # Table with columns: name (string), value (float), valid (bool) + self.results = AttrR( + Table, + structured_dtype=[ + ("name", "S32"), # 32-character string + ("value", np.float64), + ("valid", np.bool_), + ], + ) ``` ### Table Metadata @@ -113,14 +124,17 @@ from fastcs.controllers import Controller from fastcs.datatypes import Table class ChannelController(Controller): - channel_data = AttrR( - Table, - structured_dtype=[ - ("channel", np.int32), - ("temperature", np.float64), - ("status", "S10"), - ], - ) + def __init__(self): + super().__init__() + + self.channel_data = AttrR( + Table, + structured_dtype=[ + ("channel", np.int32), + ("temperature", np.float64), + ("status", "S10"), + ], + ) # Create data using numpy structured array data = np.array([ diff --git a/docs/how-to/typed-commands.md b/docs/how-to/typed-commands.md index b85f34655..563fc83eb 100644 --- a/docs/how-to/typed-commands.md +++ b/docs/how-to/typed-commands.md @@ -74,8 +74,8 @@ and results on attributes: ```python class Stage(Controller): - target = AttrRW(float) - last_position = AttrR(float) + target: AttrRW[float] + last_position: AttrR[float] @command() async def move(self) -> None: diff --git a/docs/how-to/update-attributes-from-device.md b/docs/how-to/update-attributes-from-device.md index b5b710965..f67dca33a 100644 --- a/docs/how-to/update-attributes-from-device.md +++ b/docs/how-to/update-attributes-from-device.md @@ -136,7 +136,7 @@ from fastcs.methods import scan class ChannelController(Controller): - voltage = AttrR(float) # No getter — updated by parent scan method + voltage: AttrR[float] # No getter — updated by parent scan method def __init__(self, index: int, connection): super().__init__(f"Ch{index:02d}") @@ -230,7 +230,7 @@ from fastcs.controllers import Controller class SubscriptionController(Controller): - temperature = AttrR(float) + temperature: AttrR[float] def __init__(self, subscription_client): super().__init__() diff --git a/docs/how-to/wait-methods.md b/docs/how-to/wait-methods.md index ee59bc67b..b3403cf82 100644 --- a/docs/how-to/wait-methods.md +++ b/docs/how-to/wait-methods.md @@ -13,8 +13,8 @@ from fastcs.controllers import Controller from fastcs.methods import command class MotorController(Controller): - position: AttrR[int] = AttrR(int) - target: AttrR[int] = AttrR(int) + position: AttrR[int] + target: AttrR[int] @command() async def move_and_wait(self): @@ -39,7 +39,7 @@ from fastcs.controllers import Controller from fastcs.methods import command class TemperatureController(Controller): - temperature: AttrR[float] = AttrR(float) + temperature: AttrR[float] @command() async def wait_for_stable(self): @@ -90,9 +90,9 @@ from fastcs.controllers import Controller from fastcs.methods import command class MultiAxisController(Controller): - x_position = AttrR(float) - y_position = AttrR(float) - z_position = AttrR(float) + x_position: AttrR[float] + y_position: AttrR[float] + z_position: AttrR[float] @command() async def move_all_and_wait(self): diff --git a/docs/snippets/static03.py b/docs/snippets/static03.py index 82b2ee5fb..cb490c62e 100644 --- a/docs/snippets/static03.py +++ b/docs/snippets/static03.py @@ -4,7 +4,7 @@ class TemperatureController(Controller): - device_id = AttrR(str) + device_id: AttrR[str] fastcs = FastCS(TemperatureController(), []) diff --git a/docs/snippets/static04.py b/docs/snippets/static04.py index c52801fab..724c8cccb 100644 --- a/docs/snippets/static04.py +++ b/docs/snippets/static04.py @@ -5,7 +5,7 @@ class TemperatureController(Controller): - device_id = AttrR(str) + device_id: AttrR[str] epics_ca = EpicsCATransport() diff --git a/docs/snippets/static05.py b/docs/snippets/static05.py index 2851dc6d0..b7b10f1be 100644 --- a/docs/snippets/static05.py +++ b/docs/snippets/static05.py @@ -8,7 +8,7 @@ class TemperatureController(Controller): - device_id = AttrR(str) + device_id: AttrR[str] gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") diff --git a/docs/snippets/static06.py b/docs/snippets/static06.py index f7bd33d15..98af3b41b 100644 --- a/docs/snippets/static06.py +++ b/docs/snippets/static06.py @@ -9,7 +9,7 @@ class TemperatureController(Controller): - device_id = AttrR(str) + device_id: AttrR[str] def __init__(self, settings: IPConnectionSettings): super().__init__() diff --git a/docs/tutorials/static-drivers.md b/docs/tutorials/static-drivers.md index e90248e9f..78ecf9eb5 100644 --- a/docs/tutorials/static-drivers.md +++ b/docs/tutorials/static-drivers.md @@ -58,7 +58,13 @@ The simulator has an API to get its ID. To expose this in the driver, an `Attrib be added to the `Controller`. There are 3 types of `Attribute`: `AttrR`, `AttrW` and `AttrRW`, representing the access mode of the API. The ID can be read, but it cannot be written, so add an `AttrR`. An `Attribute` also needs a type. The ID from the simulator -is a string, so `String` should be used. +is a string, so `str` should be used. + +Written as a type hint in the class body, the attribute is created for you, once per +controller instance, and is there to be referenced as soon as the controller is +constructed. Once there is a device to read it from, the same attribute gets its +`getter` - see [](../explanations/declaring-attributes.md) for the two spellings and +when to reach for each. ::::{admonition} Code 3 :class: dropdown, hint diff --git a/src/fastcs/attributes/__init__.py b/src/fastcs/attributes/__init__.py index 40f28135c..326cfb655 100644 --- a/src/fastcs/attributes/__init__.py +++ b/src/fastcs/attributes/__init__.py @@ -13,6 +13,5 @@ from .attr_w import UnboundSetter as UnboundSetter from .attribute import Attribute as Attribute from .attribute import AttributeAccessMode as AttributeAccessMode -from .hinted_attribute import HintedAttribute as HintedAttribute from .severity import Severity as Severity from .update import Update as Update diff --git a/src/fastcs/attributes/attr_r.py b/src/fastcs/attributes/attr_r.py index ce03435b2..554cb7d19 100644 --- a/src/fastcs/attributes/attr_r.py +++ b/src/fastcs/attributes/attr_r.py @@ -299,6 +299,44 @@ def severity(self) -> Severity: def has_getter(self) -> bool: return self._getter is not None + def set_getter(self, getter: Getter[DType_T] | Schedule[DType_T]) -> None: + """Provision the IO that reads this attribute, after construction. + + The `ControllerFiller` calls this to fill an attribute a class-body + hint declared but could not wire up, so that a reference taken during + ``__init__`` stays valid. It is not for changing the IO of an attribute + that already has some - that would silently swap what a running + transport is reading. + + Args: + getter: The getter, optionally wrapped in a `Polled`/`NotPolled` + schedule; a bare getter is read once, on connect + + Raises: + ValueError: If the attribute already has a getter, or a schedule + was passed with nothing to schedule + + """ + if self._getter is not None: + raise ValueError( + f"Attribute {self.full_name or type(self).__name__} already has a " + "getter" + ) + + match getter: + case Polled() | NotPolled(): + if getter.getter is None: + raise ValueError( + f"{type(getter).__name__} was given no getter to schedule" + ) + self._getter = getter.getter + self._poll_period = ( + getter.period if isinstance(getter, Polled) else None + ) + case _: + self._getter = getter + self._poll_period = ONCE + @property def poll_period(self) -> float | None: return self._poll_period diff --git a/src/fastcs/attributes/attr_w.py b/src/fastcs/attributes/attr_w.py index cd12ea4b1..b0282f02c 100644 --- a/src/fastcs/attributes/attr_w.py +++ b/src/fastcs/attributes/attr_w.py @@ -143,6 +143,27 @@ def setpoint(self) -> DType_T: def has_setter(self) -> bool: return self._setter is not None + def set_setter(self, setter: Setter[DType_T]) -> None: + """Provision the IO that writes this attribute, after construction. + + The counterpart to `AttrR.set_getter`, and used the same way: by the + `ControllerFiller`, to fill an attribute a class-body hint declared. + + Args: + setter: The setter to apply values with + + Raises: + ValueError: If the attribute already has a setter + + """ + if self._setter is not None: + raise ValueError( + f"Attribute {self.full_name or type(self).__name__} already has a " + "setter" + ) + + self._setter = setter + @property def access_mode(self) -> AttributeAccessMode: return "w" diff --git a/src/fastcs/attributes/hinted_attribute.py b/src/fastcs/attributes/hinted_attribute.py deleted file mode 100644 index d58fadd46..000000000 --- a/src/fastcs/attributes/hinted_attribute.py +++ /dev/null @@ -1,18 +0,0 @@ -from dataclasses import dataclass - -from fastcs.attributes.attribute import Attribute -from fastcs.datatypes import DType - - -@dataclass(kw_only=True) -class HintedAttribute: - """An `Attribute` type hint found on a `Controller` class - - e.g. ``attr: AttrR[int]`` - - """ - - attr_type: type[Attribute] - """The type of the `Attribute` in the type hint - e.g. `AttrR`""" - dtype: type[DType] | None - """The dtype of the `Attribute` in the type hint, if any - e.g. `int`""" diff --git a/src/fastcs/controllers/__init__.py b/src/fastcs/controllers/__init__.py index e3fe4106e..13e9de6de 100644 --- a/src/fastcs/controllers/__init__.py +++ b/src/fastcs/controllers/__init__.py @@ -2,4 +2,7 @@ from .controller import Controller as Controller from .controller_api import ControllerAPI as ControllerAPI from .controller_vector import ControllerVector as ControllerVector +from .filler import ControllerFiller as ControllerFiller +from .filler import Declaration as Declaration +from .filler import Hint as Hint from .runner import ControllerRunner as ControllerRunner diff --git a/src/fastcs/controllers/base_controller.py b/src/fastcs/controllers/base_controller.py index cf3d2e9ad..162a68a61 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -1,19 +1,13 @@ from __future__ import annotations -from copy import deepcopy from inspect import getattr_static -from typing import ( - TypeVar, - _GenericAlias, # type: ignore - get_args, - get_origin, - get_type_hints, -) - -from fastcs.attributes import Attribute, HintedAttribute, UnboundAttr +from typing import TypeVar + +from fastcs.attributes import Attribute, UnboundAttr from fastcs.controllers.controller_api import ControllerAPI -from fastcs.logging import logger -from fastcs.methods import Command, Method, Scan, UnboundCommand, UnboundScan +from fastcs.controllers.filler import ControllerFiller +from fastcs.datatypes import resolve_datatype +from fastcs.methods import Command, Scan, UnboundCommand, UnboundScan from fastcs.tracer import Tracer T = TypeVar("T") @@ -55,42 +49,12 @@ def __init__( self.__command_methods: dict[str, Command] = {} self.__scan_methods: dict[str, Scan] = {} - self.__hinted_attributes: dict[str, HintedAttribute] = {} - self.__hinted_methods: dict[str, type[Method]] = {} - self.__hinted_sub_controllers: dict[str, type[BaseController]] = {} - self._find_type_hints() + self.filler = ControllerFiller(self) + """Creates and tracks the children this controller's class body declares""" + self.filler.read_hints() self._bind_attrs() - - def _find_type_hints(self): - """Find `Attribute` and `Controller` type hints for introspection validation""" - for name, hint in get_type_hints(type(self)).items(): - if isinstance(hint, _GenericAlias): # e.g. AttrR[int] - args = get_args(hint) - hint = get_origin(hint) - else: - args = None - - if isinstance(hint, type) and issubclass(hint, Attribute): - if args is None: - dtype = None - else: - if len(args) == 1: - dtype = args[0] - else: - raise TypeError( - f"Invalid type hint for attribute {name}: {hint}" - ) - - self.__hinted_attributes[name] = HintedAttribute( - attr_type=hint, dtype=dtype - ) - - 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 + self.filler.create_children_from_hints() @classmethod def _walk_mro(cls): @@ -110,7 +74,7 @@ def _walk_mro(cls): return class_dir def _bind_attrs(self) -> None: - """Bind Attributes and Methods to this instance. + """Bind the class body's declarations to this instance. This method will bind the attributes of this controller class to this specific instance. For Attributes, this is just a case of copying and @@ -123,15 +87,16 @@ class method and a controller instance, so that it can be called from any same way the Methods are - into a fresh Attribute whose getter and setter are methods of this instance. - """ - class_dir = dict.fromkeys(self._walk_mro()) - class_type_hints = { - key: value - for key, value in get_type_hints(type(self)).items() - if not key.startswith("_") - } + - an ``@attr``-decorated getter is an `UnboundAttr`, bound into an + ``AttrR``/``AttrRW`` whose getter and setter are methods of this + instance + - a ``@command``/``@scan`` method is bound the same way, so it can be + called from any context with this instance as ``self`` - for attr_name in {**class_dir, **class_type_hints}: + Bare type hints are the third kind, and `ControllerFiller` handles + those separately - they create children rather than binding them. + """ + for attr_name in dict.fromkeys(self._walk_mro()): if attr_name == "root_attribute": continue @@ -144,22 +109,28 @@ class method and a controller instance, so that it can be called from any attr = getattr(self, attr_name, None) if isinstance(attr, Attribute): - setattr(self, attr_name, deepcopy(attr)) - else: - if isinstance(attr, Command): - self.add_command(attr_name, attr) - elif isinstance(attr, Scan): - self.add_scan(attr_name, attr) - elif isinstance( - unbound_command := getattr(attr, "__unbound_command__", None), - UnboundCommand, - ): - self.add_command(attr_name, unbound_command.bind(self)) - elif isinstance( - unbound_scan := getattr(attr, "__unbound_scan__", None), - UnboundScan, - ): - self.add_scan(attr_name, unbound_scan.bind(self)) + raise TypeError( + f"{type(self).__name__}.{attr_name} is an " + f"{type(attr).__name__} in the class body, which would be " + "shared by every instance of this controller. Construct it in " + "`__init__`, or declare it as a type hint " + f"(`{attr_name}: {type(attr).__name__}[...]`) and let the " + "filler create it." + ) + if isinstance(attr, Command): + self.add_command(attr_name, attr) + elif isinstance(attr, Scan): + self.add_scan(attr_name, attr) + elif isinstance( + unbound_command := getattr(attr, "__unbound_command__", None), + UnboundCommand, + ): + self.add_command(attr_name, unbound_command.bind(self)) + elif isinstance( + unbound_scan := getattr(attr, "__unbound_scan__", None), + UnboundScan, + ): + self.add_scan(attr_name, unbound_scan.bind(self)) def __repr__(self): name = self.__class__.__name__ @@ -186,73 +157,19 @@ async def initialise(self): def post_initialise(self): """Hook to call after all attributes added, before serving the application""" - self._validate_type_hints() - - def _validate_type_hints(self): - """Validate all 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 + self.check_filled() - def _validate_hinted_member(self, name: str, expected_type: type[T]) -> T: - """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() - return member + def check_filled(self): + """Check that every class-body declaration was provisioned, recursively. - 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: - raise RuntimeError( - f"Controller `{self.__class__.__name__}` failed to introspect " - f"hinted method `{name}` during initialisation" - ) from None - - 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""" - try: - attr = self._validate_hinted_member(name, Attribute) - except RuntimeError: - raise RuntimeError( - f"Controller `{self.__class__.__name__}` failed to introspect " - f"hinted attribute `{name}` during initialisation" - ) from None - - logger.debug( - "Validated hinted attribute", name=name, controller=self, attribute=attr - ) + A driver may call ``self.filler.check_filled()`` itself at the end of + its own ``initialise``; the framework calls this afterwards so that a + controller which forgot to does not serve a half-built API. + """ + self.filler.check_filled() - def _validate_hinted_controller(self, name: str): - """Check that a sub controller with the given name exists on the controller""" - try: - controller = self._validate_hinted_member(name, BaseController) - except RuntimeError: - raise RuntimeError( - f"Controller `{self.__class__.__name__}` failed to introspect " - f"hinted controller `{name}` during initialisation" - ) from None - - logger.debug( - "Validated hinted sub controller", - name=name, - controller=self, - sub_controller=controller, - ) + for sub_controller in self.sub_controllers.values(): + sub_controller.check_filled() @property def path(self) -> list[str]: @@ -287,21 +204,13 @@ def add_attribute(self, name, attr: Attribute): 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( - f"Controller '{self.__class__.__name__}' introspection of " - f"hinted attribute '{name}' does not match defined access mode. " - f"Expected '{hint.attr_type.__name__}' got '{type(attr).__name__}'." - ) - if hint.dtype is not None and hint.dtype != attr.dtype: - raise RuntimeError( - f"Controller '{self.__class__.__name__}' introspection of " - f"hinted attribute '{name}' does not match defined datatype. " - f"Expected '{hint.dtype.__name__}', " - f"got '{attr.dtype.__name__}'." - ) + try: + self._check_against_declaration(name, attr) + except RuntimeError as exc: + raise RuntimeError( + f"Cannot add attribute {attr}: it does not match the access mode " + f"or datatype hinted for '{name}' - {exc}." + ) from exc attr.set_name(name) attr.set_path(self.path) @@ -318,15 +227,13 @@ def add_sub_controller(self, name: str, sub_controller: BaseController): 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( - f"Controller '{self.__class__.__name__}' introspection of " - f"hinted sub controller '{name}' does not match defined type. " - f"Expected '{hint.__name__}' got " - f"'{sub_controller.__class__.__name__}'." - ) + try: + self._check_against_declaration(name, sub_controller) + except RuntimeError as exc: + raise RuntimeError( + f"Cannot add sub controller {sub_controller}: it does not match " + f"the type hinted for '{name}' - {exc}." + ) from exc sub_controller.set_path(self.path + [name]) self.__sub_controllers[name] = sub_controller @@ -339,23 +246,54 @@ def add_sub_controller(self, name: str, sub_controller: BaseController): def sub_controllers(self) -> dict[str, BaseController]: return self.__sub_controllers - def _validate_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 _check_against_declaration(self, name: str, member: object): + """Check a member being added matches what the class body declared. + + The filler creates what it can from a hint, so what reaches here is + either introspection satisfying a promise (``state: AttrR``) or a + driver adding something that clashes with a declaration. + + Raises: + RuntimeError: Saying what was declared and what arrived. The caller + is what knows which kind of member this is, so it catches this + and re-raises with that context. + + """ + declaration = self.filler.declarations.get(name) + if declaration is None: + return + + declared = declaration.hint.declared_type + if not isinstance(member, declared): + raise RuntimeError( + f"expected '{declared.__name__}', got '{type(member).__name__}'" + ) + + if declaration.hint.datatype is None or not isinstance(member, Attribute): + return + + # The hint holds the datatype as it was written, so `AttrR[Array1D[np.int32]]` + # has to be resolved to the `np.ndarray` an attribute reports. + declared_datatype, _ = resolve_datatype(declaration.hint.datatype) + if declared_datatype != member.dtype: + raise RuntimeError( + f"expected datatype '{declared_datatype.__name__}', " + f"got '{member.dtype.__name__}'" + ) def add_command(self, name: str, command: Command): try: self._check_for_name_clash(name) - self._validate_method(name, command) - except (ValueError, RuntimeError) as exc: - raise exc.__class__(f"Cannot add command method {command}.") from exc + except ValueError as exc: + raise ValueError(f"Cannot add command method {command}.") from exc + + try: + self._check_against_declaration(name, command) + except RuntimeError as exc: + raise RuntimeError( + f"Cannot add command method {command}: it does not match the type " + f"hinted for '{name}' - {exc}." + ) from exc self.__command_methods[name] = command super().__setattr__(name, command) @@ -367,9 +305,16 @@ def command_methods(self) -> dict[str, Command]: def add_scan(self, name: str, scan: Scan): try: self._check_for_name_clash(name) - self._validate_method(name, scan) - except (ValueError, RuntimeError) as exc: - raise exc.__class__(f"Cannot add scan method {scan}.") from exc + except ValueError as exc: + raise ValueError(f"Cannot add scan method {scan}.") from exc + + try: + self._check_against_declaration(name, scan) + except RuntimeError as exc: + raise RuntimeError( + f"Cannot add scan method {scan}: it does not match the type " + f"hinted for '{name}' - {exc}." + ) from exc self.__scan_methods[name] = scan super().__setattr__(name, scan) diff --git a/src/fastcs/controllers/filler.py b/src/fastcs/controllers/filler.py new file mode 100644 index 000000000..3a53495ad --- /dev/null +++ b/src/fastcs/controllers/filler.py @@ -0,0 +1,382 @@ +"""Create a `Controller`'s children from the type hints in its class body. + +The declarative half of ADR 0013: a class body holds *declarations*, and the +data that turns a declaration into a working attribute arrives later - from a +device's own description of itself, or from a protocol library's static +metadata. `ControllerFiller` is what stands between the two. It is a structural +port of ophyd-async's ``DeviceFiller``; the names follow FastCS's vocabulary +rather than ophyd-async's. + +A hint that says what it holds is **created unfilled** while the controller is +still constructing:: + + class OdinDetector(Controller): + frames: AttrRW[int] # exists as soon as __init__ returns + + async def initialise(self) -> None: + self.filler.fill_attribute("frames", getter=..., setter=...) + +so ``self.frames`` can be referenced by the rest of ``__init__`` - the rule +ADR 0013 takes from ophyd-async, and what makes ``initialise`` safe to run in +parallel across controllers. Filling provisions the IO and metadata on the +attribute that is already there, so a reference taken during ``__init__`` stays +valid. + +A hint that cannot say what it holds - ``state: AttrR``, where the datatype is +an enum whose members only exist on the wire - is a **promise** instead: it is +not created, and `ControllerFiller.check_filled` requires that introspection +added it by the time the controller is initialised. +""" + +from __future__ import annotations + +import types +from collections.abc import Callable, Iterator +from dataclasses import dataclass, field +from functools import partial +from typing import ( + TYPE_CHECKING, + Any, + Union, + Unpack, + get_args, + get_origin, + get_type_hints, +) + +from fastcs.attributes import Attribute, AttrR, AttrW +from fastcs.attributes.attr_r import Getter, Schedule +from fastcs.attributes.attr_w import Setter +from fastcs.datatypes import DType, DType_T, Meta, validate_meta +from fastcs.methods import Method + +if TYPE_CHECKING: + from fastcs.controllers.base_controller import BaseController + + +@dataclass +class Hint: + """A class-body hint, taken apart.""" + + type_: Any + """The hint with ``Annotated``/``| None`` stripped - ``AttrR[int]``""" + declared_type: Any + """What the hint is a subscript of - ``AttrR``, and ``AttrR`` for a bare + ``AttrR`` too. Not necessarily a class: a ``list[int] | str`` hint leaves a + ``typing.Union`` here, which is why the filler checks before using it.""" + datatype: type[DType] | None = None + """The datatype the hint subscripts its class with - ``int`` for + ``AttrR[int]`` - or ``None`` for a hint that does not say what it holds""" + extras: tuple[Any, ...] = field(default_factory=tuple) + """Whatever else an ``Annotated`` hint carried, for a protocol layer to read""" + optional: bool = False + """Whether the hint was ``| None``, so `ControllerFiller.check_filled` does + not require it""" + + +@dataclass +class Declaration: + """One class-body hint the filler found, and what became of it.""" + + name: str + """The attribute name on the controller, with any trailing underscore gone""" + raw_name: str + """The name as the class body wrote it, trailing underscore and all""" + hint: Hint + """The hint itself - what it declared, what it holds, and what it carried""" + child: Attribute | None = None + """The unfilled `Attribute` this created, if it could create one""" + + +def _unwrap(hint: Any) -> Hint: + """Strip ``Annotated`` and ``| None`` off a hint, keeping what they carried.""" + extras: tuple[Any, ...] = () + optional = False + + # Annotated[X, ...] carries its extras on __metadata__, and X on the origin. + metadata = getattr(hint, "__metadata__", None) + if metadata is not None: + extras = tuple(metadata) + hint = hint.__origin__ + + if get_origin(hint) in (Union, types.UnionType): + args = [arg for arg in get_args(hint) if arg is not type(None)] + optional = len(args) != len(get_args(hint)) + if len(args) == 1: + hint = args[0] + # Annotated may sit inside the union rather than outside it. + inner = _unwrap(hint) + hint, extras = inner.type_, extras or inner.extras + + return Hint( + type_=hint, + declared_type=get_origin(hint) or hint, + datatype=_datatype_of(hint), + extras=extras, + optional=optional, + ) + + +def _datatype_of(hint: Any) -> type[DType] | None: + """The datatype an ``AttrR[int]``-style hint declares, or ``None``. + + ``None`` means the hint named an attribute class without saying what it + holds, which is a promise rather than something the filler can build. + """ + args = get_args(hint) + return args[0] if len(args) == 1 else None + + +class ControllerFiller: + """Creates and tracks the children a `Controller`'s class body declares. + + Every controller has one, as ``controller.filler``. It reads the class + hints once, during construction, and holds what it found until + `check_filled` reports on it. + """ + + def __init__(self, controller: BaseController) -> None: + self._controller = controller + self._declarations: dict[str, Declaration] = {} + + def read_hints(self) -> None: + """Record what the class body declares, without creating anything yet. + + Called by ``BaseController.__init__`` before the ``@attr``/``@command`` + declarations are bound, so that what those bind can be checked against + a hint of the same name. + """ + from fastcs.controllers.base_controller import BaseController + + for raw_name, raw_hint in get_type_hints( + type(self._controller), include_extras=True + ).items(): + if raw_name.startswith("_") or raw_name == "root_attribute": + # `root_attribute` is what a parent shows for this controller + # rather than an attribute of it, and is declared on + # `BaseController` itself, so it is not the filler's to create. + continue + + hint = _unwrap(raw_hint) + + # What a hint declares is the class it subscripts, but not every + # hint has one: `power: Annotated[AttrRW[float], spec] | AttrRW[int]` + # unwraps to a `typing.Union`, and `power: dict[str, int]` to a + # `dict`. The `issubclass` below is what rejects the second, and it + # raises `TypeError` rather than returning False for the first, so + # anything that is not a class is dropped before it gets there. + if not isinstance(hint.declared_type, type): + continue + + if not issubclass(hint.declared_type, Attribute | Method | BaseController): + continue + + # ophyd-async's convention: a trailing underscore keeps a name that + # would otherwise shadow a builtin or a framework member off the + # class body, without renaming the attribute it declares. + name = raw_name.removesuffix("_") + + self._declarations[name] = Declaration( + name=name, raw_name=raw_name, hint=hint + ) + + def create_children_from_hints(self) -> None: + """Create an unfilled `Attribute` for every hint that can produce one. + + Called once by ``BaseController.__init__``, after the class body's + declarations have been bound. A name cannot be declared both by an + attribute decorator and by a type hint. + """ + for declaration in self._declarations.values(): + if not issubclass(declaration.hint.declared_type, Attribute): + continue + + if declaration.name in self._controller.attributes: + raise TypeError( + f"Controller `{type(self._controller).__name__}` cannot " + f"declare '{declaration.name}' with both an attribute " + "decorator and a type hint" + ) + + self._create_attribute(declaration) + + def _create_attribute(self, declaration: Declaration) -> None: + if declaration.hint.datatype is None: + # A hint that does not say what it holds cannot be built, only + # promised. `state: AttrR` on an introspecting controller is the + # motivating case - the enum's members are only known over the wire. + return + + attr_type: type[Attribute] = declaration.hint.declared_type + attribute = attr_type(declaration.hint.datatype) + declaration.child = attribute + self._controller.add_attribute(declaration.name, attribute) + + @property + def declarations(self) -> dict[str, Declaration]: + """Every class-body declaration this filler found, by name.""" + return self._declarations + + def __iter__(self) -> Iterator[tuple[Attribute | None, tuple[Any, ...]]]: + """Yield ``(child, extras)`` for each declaration, as ADR 0013 asks. + + ``child`` is the unfilled `Attribute` where one could be created, and + ``None`` for a promise. ``extras`` is whatever an ``Annotated`` hint + carried, which is how a protocol library outside core FastCS - an SCPI + package, say - gets its own declaration vocabulary without FastCS + knowing anything about it. + """ + for declaration in self._declarations.values(): + yield declaration.child, declaration.hint.extras + + def fill_attribute( + self, + name: str, + getter: Getter[DType_T] | Schedule[DType_T] | None = None, + setter: Setter[DType_T] | None = None, + datatype: type[DType_T] | None = None, + **meta: Unpack[Meta], + ) -> Attribute: + """Provision a declared attribute with its IO and metadata. + + Args: + name: The name the class body declared + getter: IO to read the value with, optionally wrapped in a + `Polled`/`NotPolled` schedule + setter: IO to write the value with + datatype: What the filling data says the attribute holds, checked + against what the hint declared. Pass it when the source could + disagree - introspection of a device that has changed under + you - and leave it out when it cannot. + meta: Metadata for the attribute, validated against the datatype + the hint declared - ``precision`` on a ``str`` raises + + Returns: + The attribute, which is the same object the hint created + + Raises: + KeyError: If nothing of that name was declared + TypeError: If the attribute has no half the given IO would fill, + the datatype disagrees with the hint, or the metadata does not + suit the datatype + + """ + declaration = self._declarations.get(name) + if declaration is None: + raise KeyError( + f"{type(self._controller).__name__} has no attribute declaration " + f"named '{name}' to fill. Declare it as a class-body hint with its " + "datatype, or add the attribute with `add_attribute`." + ) + + if declaration.child is None: + # A hint that does not name its datatype - `state: AttrR` - is a + # promise rather than something the filler could build, so there is + # no attribute here to provision. + raise KeyError( + f"{type(self._controller).__name__} declared '{name}' as " + f"{declaration.hint.type_} without a datatype, so there is no " + "attribute to fill. Subscript the hint with the datatype it holds, " + "or add the attribute with `add_attribute`." + ) + + attribute = declaration.child + + # The whole request is checked before any of it is applied, so that a + # rejected fill leaves the attribute exactly as it was. Otherwise a bad + # setter would land its getter first, and the corrected call would be + # refused by `set_getter` for IO the failed one had installed. + if datatype is not None and datatype != attribute.dtype: + raise TypeError( + f"Controller '{type(self._controller).__name__}' filled hinted " + f"attribute '{name}' with the wrong datatype. Expected " + f"'{attribute.dtype.__name__}', got " + f"'{getattr(datatype, '__name__', datatype)}'." + ) + + apply: list[Callable[[], None]] = [] + + if getter is not None: + if not isinstance(attribute, AttrR): + raise TypeError( + f"Attribute '{name}' was declared " + f"{type(attribute).__name__}, which has nothing to read." + ) + if attribute.has_getter(): + raise ValueError( + f"Attribute {attribute.full_name or name} already has a getter" + ) + apply.append(partial(attribute.set_getter, getter)) + + if setter is not None: + if not isinstance(attribute, AttrW): + raise TypeError( + f"Attribute '{name}' was declared " + f"{type(attribute).__name__}, which has nothing to write." + ) + if attribute.has_setter(): + raise ValueError( + f"Attribute {attribute.full_name or name} already has a setter" + ) + apply.append(partial(attribute.set_setter, setter)) + + if meta: + # `validate_meta` is what `update_meta` runs before it assigns, and + # the runtime counterpart to the static `Unpack[FloatMeta]` check on + # the constructors. + merged: Meta = {**attribute.meta, **meta} + validate_meta(attribute.dtype, merged, attribute.full_name or name) + apply.append(partial(attribute.update_meta, merged)) + + for step in apply: + step() + + return attribute + + def fill_meta(self, name: str, meta: Meta) -> Attribute: + """Fill a declared attribute's metadata from an extras object. + + The shape a protocol layer wants: ``SCPIParam(...).meta`` in one go, + validated against the datatype the hint declared. + """ + return self.fill_attribute(name, **meta) + + def check_filled(self) -> None: + """Raise if anything the class body promised does not exist. + + A declaration the filler could create is satisfied by having been + created, so what this reports is the promised-but-missing: a hint whose + datatype was not knowable at author time, which introspection was + therefore expected to add and did not. An ``| None`` hint is not + required. + + Raises: + RuntimeError: Listing, by name, what is still missing + + """ + missing: list[str] = [] + + for name, declaration in self._declarations.items(): + if declaration.hint.optional: + continue + + declared = declaration.hint.declared_type + member = getattr(self._controller, name, None) + if isinstance(member, declared): + continue + + if member is None: + missing.append(f"{name} (declared {declared.__name__}, never added)") + else: + missing.append( + f"{name} (declared {declared.__name__}, " + f"got {type(member).__name__})" + ) + + if not missing: + return + + raise RuntimeError( + f"Controller `{type(self._controller).__name__}` did not provision: " + + ", ".join(sorted(missing)) + ) diff --git a/src/fastcs/demo/eiger.py b/src/fastcs/demo/eiger.py index ba89d1f13..afebc0c7c 100644 --- a/src/fastcs/demo/eiger.py +++ b/src/fastcs/demo/eiger.py @@ -111,7 +111,7 @@ class EigerDetector(Controller): # Derived (soft): built on top of the introspected ``state`` param. Declaring # ``state`` as a checked attribute is what lets us reference it in code and # publish something computed from it - here, whether the detector is idle. - idle = AttrR(bool) + idle: AttrR[bool] def __init__( self, @@ -153,22 +153,36 @@ async def initialise(self) -> None: datatype = _datatype(param, data) if data["access_mode"] == "rw": - attr = AttrRW( - datatype, - getter=self._getter(subsystem, param), - setter=self._setter(subsystem, param), - ) + getter = self._getter(subsystem, param) + setter = self._setter(subsystem, param) else: # Read-only params are status values that change on the device, # so poll them periodically rather than reading once. - attr = AttrR( - datatype, - getter=Polled( - self._getter(subsystem, param), period=UPDATE_PERIOD - ), + getter = Polled( + self._getter(subsystem, param), period=UPDATE_PERIOD + ) + setter = None + + declaration = self.filler.declarations.get(param) + if declaration is not None and declaration.child is not None: + # A parameter the class body declared already exists as an + # unfilled attribute, so provision that one rather than + # adding a second of the same name. The filler checks the + # access mode and datatype the hint promised against what + # the device turned out to report. + self.filler.fill_attribute( + param, datatype=datatype, getter=getter, setter=setter + ) + elif setter is None: + self.add_attribute(param, AttrR(datatype, getter=getter)) + else: + self.add_attribute( + param, AttrRW(datatype, getter=getter, setter=setter) ) - self.add_attribute(param, attr) + # Every hinted parameter should have turned up in the tree the device + # reported. + self.filler.check_filled() # Keep the derived ``idle`` flag in sync with the introspected ``state``. self.state.add_readback_callback(self._update_idle) diff --git a/tests/benchmarking/controller.py b/tests/benchmarking/controller.py index 19932655b..46126bcbe 100644 --- a/tests/benchmarking/controller.py +++ b/tests/benchmarking/controller.py @@ -10,8 +10,11 @@ class MyTestController(Controller): - read_int: AttrR = AttrR(int, initial_value=0) - write_bool: AttrW = AttrW(bool) + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.read_int = AttrR(int, initial_value=0) + + write_bool: AttrW[bool] def run(): diff --git a/tests/conftest.py b/tests/conftest.py index 937526e78..569b863e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -41,12 +41,12 @@ def clear_softioc_records(): class BackendTestController(MyTestController): - read_int: AttrR = AttrR(int) - read_write_int: AttrRW = AttrRW(int) - read_write_float: AttrRW = AttrRW(float) - read_bool: AttrR = AttrR(bool) - write_bool: AttrW = AttrW(bool) - read_string: AttrRW = AttrRW(str) + read_int: AttrR[int] + read_write_int: AttrRW[int] + read_write_float: AttrRW[float] + read_bool: AttrR[bool] + write_bool: AttrW[bool] + read_string: AttrRW[str] @pytest.fixture diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index f41b74d6c..1aa5816e0 100644 --- a/tests/example_p4p_ioc.py +++ b/tests/example_p4p_ioc.py @@ -22,27 +22,38 @@ class FEnum(enum.Enum): class ParentController(Controller): - description = "some controller" - a: AttrRW = AttrRW( - int, - limits=NumericLimits(control=Limits(high=400_000), alarm=Limits(high=40_000)), - ) - b: AttrW = AttrW( - float, limits=NumericLimits(control=Limits(low=-1), alarm=Limits(low=-0.5)) - ) + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.a = AttrRW( + int, + limits=NumericLimits( + control=Limits(high=400_000), alarm=Limits(high=40_000) + ), + ) + self.b = AttrW( + float, limits=NumericLimits(control=Limits(low=-1), alarm=Limits(low=-0.5)) + ) + self.table = AttrRW( + Table, + structured_dtype=[ + ("A", np.int32), + ("B", "i"), + ("C", "?"), + ("D", np.float64), + ], + ) - table: AttrRW = AttrRW( - Table, - structured_dtype=[("A", np.int32), ("B", "i"), ("C", "?"), ("D", np.float64)], - ) + description = "some controller" class ChildController(Controller): fail_on_next_e = True - c: AttrW = AttrW(int) + c: AttrW[int] def __init__(self, description: str | None = None): super().__init__(description=description) + self.g = AttrRW(Array1D[np.int64], shape=(3,)) + self.h = AttrRW(Array1D[np.float64], shape=(3, 3)) # A getter/setter pair against an in-memory "device", doing what an # AttributeIO used to do. The setter clamps the requested value and @@ -65,15 +76,13 @@ async def d(self): print("D: FINISHED") await self.j.update(self.j.readback + 1) - e: AttrR = AttrR(bool) + e: AttrR[bool] @scan(1) async def flip_flop(self): await self.e.update(not self.e.readback) - f: AttrRW = AttrRW(FEnum) - g: AttrRW = AttrRW(Array1D[np.int64], shape=(3,)) - h: AttrRW = AttrRW(Array1D[np.float64], shape=(3, 3)) + f: AttrRW[FEnum] @command() async def i(self): @@ -87,7 +96,7 @@ async def i(self): print("I: FINISHED") await self.j.update(self.j.readback + 1) - j: AttrR = AttrR(int) + j: AttrR[int] def run(id="P4P_TEST_DEVICE"): @@ -97,10 +106,9 @@ def run(id="P4P_TEST_DEVICE"): controller.set_path([id]) class ChildVector(ControllerVector): - vector_attribute: AttrR = AttrR(int) - def __init__(self, children, description=None): super().__init__(children, description) + self.vector_attribute = AttrR(int) sub_controller = ChildVector( { diff --git a/tests/example_softioc.py b/tests/example_softioc.py index 88da09e1e..e3cb1f054 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -12,8 +12,8 @@ class ParentController(Controller): - a: AttrR = AttrR(int) - b: AttrRW = AttrRW(int) + a: AttrR[int] + b: AttrRW[int] def __init__(self, description: str | None = None) -> None: super().__init__(description) @@ -29,7 +29,7 @@ async def set_clamped(self, value: int) -> int: class ChildController(Controller): - c: AttrW = AttrW(int) + c: AttrW[int] @command() async def d(self): diff --git a/tests/test_attr_decorator.py b/tests/test_attr_decorator.py index abf711496..f5c5c1474 100644 --- a/tests/test_attr_decorator.py +++ b/tests/test_attr_decorator.py @@ -266,20 +266,7 @@ class Device(Controller): async def label(self) -> str: return "x" - controller = Device() - - assert isinstance(controller.label, AttrR) - - -def test_type_hint_of_the_wrong_access_mode_raises(): - class Device(Controller): - label: AttrRW[str] # pyright: ignore[reportRedeclaration] - - @AttrR.declare - async def label(self) -> str: - return "x" - - with pytest.raises(RuntimeError, match="does not match defined access mode"): + with pytest.raises(TypeError, match="both an attribute decorator and a type hint"): Device() diff --git a/tests/test_controller_filler.py b/tests/test_controller_filler.py new file mode 100644 index 000000000..f52cd9a8e --- /dev/null +++ b/tests/test_controller_filler.py @@ -0,0 +1,329 @@ +"""Tests for `ControllerFiller` - the declarative half of ADR 0013.""" + +import enum +from collections.abc import Callable +from typing import Annotated + +import numpy as np +import pytest + +from fastcs.attributes import AttrR, AttrRW, AttrW, NotPolled +from fastcs.controllers import BaseController, Controller, ControllerVector +from fastcs.datatypes import Array1D + + +class Colour(enum.Enum): + RED = "red" + + +def test_every_attribute_datatype_can_be_declared(): + class Declared(Controller): + flag: AttrR[bool] + count: AttrRW[int] + reading: AttrR[float] + label: AttrRW[str] + colour: AttrR[Colour] + trace: AttrR[Array1D[np.int32]] + + controller = Declared() + + assert {name: attr.dtype for name, attr in controller.attributes.items()} == { + "flag": bool, + "count": int, + "reading": float, + "label": str, + "colour": Colour, + "trace": np.ndarray, + } + + +def test_access_mode_comes_from_the_declared_class(): + class Declared(Controller): + readable: AttrR[int] + writable: AttrW[int] + both: AttrRW[int] + + controller = Declared() + + assert controller.readable.access_mode == "r" + assert controller.writable.access_mode == "w" + assert controller.both.access_mode == "rw" + + +@pytest.mark.asyncio +async def test_attributes_added_without_a_hint(): + # `fastcs-PandABlocks`/`fastcs-secop`: the whole tree comes off the wire. + class Dynamic(Controller): + async def initialise(self) -> None: + self.add_attribute("discovered", AttrR(int)) + + controller = Dynamic() + + assert controller.attributes == {} + + await controller.initialise() + + assert set(controller.attributes) == {"discovered"} + controller.check_filled() + + +def test_a_trailing_underscore_is_dropped_from_the_name(): + # ophyd-async's convention for a name that would otherwise clash. + class Declared(Controller): + description_: AttrR[str] + + controller = Declared() + + assert "description" in controller.attributes + + +def test_extras_from_an_annotated_hint_are_handed_back(): + class SCPIParam: + def __init__(self, param: str) -> None: + self.param = param + + spec = SCPIParam("P") + + class Declared(Controller): + power: Annotated[AttrRW[float], spec] + + controller = Declared() + + assert controller.filler.declarations["power"].hint.extras == (spec,) + assert list(controller.filler) == [(controller.power, (spec,))] + + +def test_extras_survive_an_optional_annotated_hint(): + spec = object() + + class Declared(Controller): + maybe: Annotated[AttrR[int], spec] | None + + declaration = Declared().filler.declarations["maybe"] + + assert declaration.hint.extras == (spec,) + assert declaration.hint.optional + + +class Child(Controller): + pass + + +class ChildHintedParent(Controller): + child: Child + + +class VectorHintedParent(Controller): + children: ControllerVector[Child] + + +@pytest.mark.parametrize( + "parent_type, expected", + [ + (ChildHintedParent, "child .declared Child, never added."), + (VectorHintedParent, "children .declared ControllerVector"), + ], +) +def test_a_controller_hint_is_not_created(parent_type: type[Controller], expected: str): + controller = parent_type() + + assert controller.sub_controllers == {} + + with pytest.raises(RuntimeError, match=expected): + controller.check_filled() + + +@pytest.mark.parametrize( + "parent_type, name, child", + [ + (ChildHintedParent, "child", Child), + (VectorHintedParent, "children", lambda: ControllerVector({1: Child()})), + ], +) +def test_adding_a_promised_controller_satisfies_the_declaration( + parent_type: type[Controller], name: str, child: Callable[[], BaseController] +): + controller = parent_type() + + controller.add_sub_controller(name, child()) + + controller.check_filled() + + +def test_check_filled_reports_every_missing_declaration_at_once(): + class Declared(Controller): + one: AttrR + two: AttrR + + with pytest.raises(RuntimeError, match="one .*, two .*"): + Declared().check_filled() + + +def test_check_filled_recurses_into_sub_controllers(): + class Child(Controller): + promised: AttrR + + class Parent(Controller): + def __init__(self) -> None: + super().__init__() + self.child = Child() + + with pytest.raises(RuntimeError, match="promised .declared AttrR"): + Parent().check_filled() + + +def test_a_class_body_attribute_instance_is_rejected(): + class Shared(Controller): + attr = AttrR(int) + + with pytest.raises(TypeError, match="Shared.attr is an AttrR in the class body"): + Shared() + + +def test_a_decorated_attribute_cannot_satisfy_a_hint_of_the_same_name(): + class Declared(Controller): + voltage: AttrR[float] # pyright: ignore[reportRedeclaration] + + @AttrR.declare + async def voltage(self) -> float: + return 1.5 + + with pytest.raises(TypeError, match="both an attribute decorator and a type hint"): + Declared() + + +def test_a_decorated_attribute_disagreeing_with_its_hint_raises(): + class Declared(Controller): + voltage: AttrR[int] # pyright: ignore[reportRedeclaration] + + @AttrR.declare + async def voltage(self) -> float: + return 1.5 + + with pytest.raises(RuntimeError, match="expected datatype 'int', got 'float'"): + Declared() + + +@pytest.mark.asyncio +async def test_filling_only_a_getter_leaves_a_read_only_attribute_readable(): + class Declared(Controller): + reading: AttrR[float] + + controller = Declared() + + async def get() -> float: + return 2.5 + + controller.filler.fill_attribute("reading", getter=NotPolled(get)) + + assert controller.reading.poll_period is None + assert await controller.reading.poll() == 2.5 + + +def test_filling_a_setter_on_a_read_only_attribute_raises(): + class Declared(Controller): + reading: AttrR[float] + + controller = Declared() + + async def put(value: float) -> None: + pass + + with pytest.raises(TypeError, match="nothing to write"): + controller.filler.fill_attribute("reading", setter=put) + + +def test_filling_a_getter_on_a_write_only_attribute_raises(): + class Declared(Controller): + demand: AttrW[float] + + controller = Declared() + + async def get() -> float: + return 0.0 + + with pytest.raises(TypeError, match="nothing to read"): + controller.filler.fill_attribute("demand", getter=get) + + +def test_filling_twice_raises(): + class Declared(Controller): + reading: AttrR[float] + + controller = Declared() + + async def get() -> float: + return 0.0 + + controller.filler.fill_attribute("reading", getter=get) + + with pytest.raises(ValueError, match="already has a getter"): + controller.filler.fill_attribute("reading", getter=get) + + +@pytest.mark.asyncio +async def test_a_rejected_fill_leaves_the_attribute_unfilled(): + # So the corrected call is not refused by IO the failed one had installed. + class Declared(Controller): + reading: AttrR[float] + + controller = Declared() + + async def get() -> float: + return 3.5 + + async def put(value: float) -> None: + pass + + with pytest.raises(TypeError, match="nothing to write"): + controller.filler.fill_attribute("reading", getter=get, setter=put) + + controller.filler.fill_attribute("reading", getter=get) + + assert await controller.reading.poll() == 3.5 + + +def test_a_rejected_fill_leaves_the_metadata_alone(): + class Declared(Controller): + reading: AttrR[float] + + controller = Declared() + + with pytest.raises(TypeError, match="not valid metadata"): + controller.filler.fill_attribute( + "reading", + units="mm", + structured_dtype=[("index", np.int32)], # pyright: ignore[reportCallIssue] + ) + + assert controller.reading.meta == {} + + +def test_fill_meta_takes_a_whole_meta_dict(): + class Declared(Controller): + reading: AttrR[float] + + controller = Declared() + controller.filler.fill_meta("reading", {"units": "mm", "precision": 2}) + + assert controller.reading.meta == {"units": "mm", "precision": 2} + + +def test_hinted_attributes_are_not_shared_between_instances(): + class Declared(Controller): + count: AttrRW[int] + + one, two = Declared(), Declared() + + assert one.count is not two.count + + +def test_attributes_can_be_added_to_a_bare_controller_from_outside(): + # What a filler does, and what `fastcs-catio` moves to instead of building + # Controller classes at runtime with `type(...)` - ADR 0013, question 2. + controller = Controller() + controller.add_attribute("discovered", AttrR(int)) + controller.temperature = AttrRW(float) + + assert set(controller.attributes) == {"discovered", "temperature"} + controller.check_filled() diff --git a/tests/test_controllers.py b/tests/test_controllers.py index c6d2a9ee9..fb8575c21 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -3,7 +3,7 @@ import pytest -from fastcs.attributes import AttrR, AttrRW +from fastcs.attributes import AttrR, AttrRW, AttrW, Polled from fastcs.controllers import Controller, ControllerVector from fastcs.methods import Command, Scan, command, scan @@ -32,15 +32,15 @@ class SomeSubController(Controller): def __init__(self): super().__init__() - sub_attribute = AttrR(int) + sub_attribute: AttrR[int] root_attribute = AttrR(int) class SomeController(Controller): annotated_attr_not_defined_in_init: AttrR[int] - equal_attr = AttrR(int) - annotated_and_equal_attr: AttrR[int] = AttrR(int) + equal_attr: AttrR[int] + annotated_and_equal_attr: AttrR[int] def __init__(self, sub_controller: Controller): super().__init__() @@ -61,16 +61,17 @@ def test_attribute_parsing(): "_attributes_attr", "attr_on_object", "_attributes_attr_equal", + "annotated_attr_not_defined_in_init", "annotated_and_equal_attr", "equal_attr", "sub_controller", } - assert SomeController.equal_attr is not controller.equal_attr - assert ( - SomeController.annotated_and_equal_attr - is not controller.annotated_and_equal_attr - ) + # Every hinted attribute is created for this instance alone, so two + # controllers of the same class never share one. + other = SomeController(SomeSubController()) + assert other.equal_attr is not controller.equal_attr + assert other.annotated_and_equal_attr is not controller.annotated_and_equal_attr assert sub_controller.attributes == { "sub_attribute": sub_controller.sub_attribute, @@ -99,11 +100,11 @@ 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__() + self.attr = AttrR(int) self.sub_controller = Controller() controller = ConflictingController() @@ -155,87 +156,203 @@ def test_controller_vector_iter(): assert sub_controllers[index] == child -def test_attribute_hint_validation(): +def test_a_hint_with_a_datatype_is_created_unfilled(): class HintedController(Controller): read_write_int: AttrRW[int] controller = HintedController() - with pytest.raises(RuntimeError, match="does not match defined datatype"): - controller.add_attribute("read_write_int", AttrRW(float)) + # The rule from ADR 0013: it exists as soon as __init__ returns, so the + # rest of __init__ may reference it. + assert isinstance(controller.read_write_int, AttrRW) + assert controller.read_write_int.dtype is int + assert not controller.read_write_int.has_getter() + assert not controller.read_write_int.has_setter() - with pytest.raises(RuntimeError, match="does not match defined access mode"): - controller.add_attribute("read_write_int", AttrR(int)) + controller.check_filled() - with pytest.raises(RuntimeError, match="failed to introspect hinted attribute"): - controller.read_write_int = 5 # type: ignore - controller._validate_type_hints() - with pytest.raises(RuntimeError, match="failed to introspect hinted attribute"): - controller._validate_type_hints() +@pytest.mark.asyncio +async def test_filling_a_hinted_attribute(): + class HintedController(Controller): + read_write_int: AttrRW[int] - controller.add_attribute("read_write_int", AttrRW(int)) + controller = HintedController() + attribute = controller.read_write_int + async def get() -> int: + return 7 -def test_enum_attribute_hint_validation(): - class GoodEnum(enum.IntEnum): - VAL = 0 + async def put(value: int) -> None: + pass - class BadEnum(enum.IntEnum): - VAL = 0 + controller.filler.fill_attribute( + "read_write_int", getter=Polled(get, period=0.5), setter=put, units="counts" + ) + + # Filled in place, so a reference taken during __init__ is still the one + # that ends up serving the device. + assert controller.read_write_int is attribute + assert attribute.poll_period == 0.5 + assert attribute.meta.get("units") == "counts" + assert await attribute.poll() == 7 + +def test_filling_the_wrong_datatype_raises(): class HintedController(Controller): - enum: AttrRW[GoodEnum] + read_write_int: AttrRW[int] controller = HintedController() - with pytest.raises(RuntimeError, match="does not match defined datatype"): - controller.add_attribute("enum", AttrRW(BadEnum)) + with pytest.raises(TypeError, match="wrong datatype"): + controller.filler.fill_attribute("read_write_int", datatype=float) - controller.add_attribute("enum", AttrRW(GoodEnum)) +def test_filling_metadata_the_datatype_has_no_use_for_raises(): + class HintedController(Controller): + label: AttrR[str] -@pytest.mark.asyncio -async def test_sub_controller_hint_validation(): + controller = HintedController() + + with pytest.raises(TypeError, match="'precision' is not valid metadata for str"): + controller.filler.fill_attribute("label", precision=3) + + +def test_filling_something_that_was_never_declared_raises(): class HintedController(Controller): - child: SomeSubController + read_write_int: AttrRW[int] controller = HintedController() - with pytest.raises(RuntimeError, match="failed to introspect hinted controller"): - controller._validate_type_hints() + with pytest.raises(KeyError, match="no attribute declaration"): + controller.filler.fill_attribute("not_declared") + + +class PromisedAttrController(Controller): + # The datatype is only knowable over the wire, so the filler cannot build + # this one - introspection must add it. + state: AttrR + + +def test_a_hint_without_a_datatype_is_not_created(): + controller = PromisedAttrController() + + assert "state" not in controller.attributes + + +def test_a_hint_without_a_datatype_is_promised(): + controller = PromisedAttrController() + + with pytest.raises(RuntimeError, match="state .declared AttrR, never added."): + controller.check_filled() + + +def test_adding_a_promised_attribute_with_the_wrong_access_mode_raises(): + controller = PromisedAttrController() + + with pytest.raises(RuntimeError, match="expected 'AttrR', got 'AttrW'"): + controller.add_attribute("state", AttrW(int)) + + +def test_adding_a_promised_attribute_satisfies_the_declaration(): + controller = PromisedAttrController() + + controller.add_attribute("state", AttrR(int)) + + controller.check_filled() + + +def test_an_optional_hint_is_not_required(): + class HintedController(Controller): + maybe: AttrR | None - with pytest.raises(RuntimeError, match="does not match defined type"): + HintedController().check_filled() + + +class GoodEnum(enum.IntEnum): + VAL = 0 + + +class BadEnum(enum.IntEnum): + VAL = 0 + + +class EnumHintedController(Controller): + colour: AttrRW[GoodEnum] + + +def test_filling_an_enum_attribute_with_another_enum_raises(): + controller = EnumHintedController() + + with pytest.raises(TypeError, match="wrong datatype"): + controller.filler.fill_attribute("colour", datatype=BadEnum) + + +def test_filling_an_enum_attribute_with_the_declared_enum_is_accepted(): + controller = EnumHintedController() + + controller.filler.fill_attribute("colour", datatype=GoodEnum) + + assert controller.colour.dtype is GoodEnum + + +class SubControllerHintedController(Controller): + child: SomeSubController + + +def test_a_sub_controller_hint_is_promised(): + controller = SubControllerHintedController() + + with pytest.raises(RuntimeError, match="child .declared SomeSubController"): + controller.check_filled() + + +def test_adding_a_sub_controller_of_the_wrong_type_raises(): + controller = SubControllerHintedController() + + with pytest.raises(RuntimeError, match="expected 'SomeSubController'"): controller.add_sub_controller("child", Controller()) + +def test_adding_the_declared_sub_controller_satisfies_the_declaration(): + controller = SubControllerHintedController() + controller.add_sub_controller("child", SomeSubController()) - controller._validate_type_hints() + controller.check_filled() -@pytest.mark.asyncio -async def test_method_hint_validation(): - class HintedController(Controller): - method: Scan - controller = HintedController() +class MethodHintedController(Controller): + method: Scan + + +def test_a_method_hint_is_promised(): + controller = MethodHintedController() + + with pytest.raises(RuntimeError, match="method .declared Scan, never added."): + controller.check_filled() - with pytest.raises(RuntimeError, match="failed to introspect hinted method"): - controller._validate_type_hints() - with pytest.raises(RuntimeError, match="Cannot add command method"): +def test_adding_a_method_of_the_wrong_kind_raises(): + controller = MethodHintedController() + + with pytest.raises(RuntimeError, match="expected 'Scan', got 'Command'"): controller.add_command("method", Command(noop)) + +def test_adding_the_declared_method_satisfies_the_declaration(): + controller = MethodHintedController() + controller.add_scan("method", Scan(fn=noop, period=0.1)) - controller._validate_type_hints() + controller.check_filled() def test_controller_api(): class MyTestController(Controller): - attr1: AttrRW[int] = AttrRW(int) - def __init__(self): super().__init__(description="Controller for testing") + self.attr1 = AttrRW(int) self.attr2 = AttrRW(int) diff --git a/tests/test_launch.py b/tests/test_launch.py index df7bdae58..987c6bff8 100644 --- a/tests/test_launch.py +++ b/tests/test_launch.py @@ -44,7 +44,7 @@ def __init__(self, arg): class IsHinted(Controller): - read = AttrR(int) + read: AttrR[int] def __init__(self, arg: SomeConfig) -> None: super().__init__() diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 1edf30f72..7b561773f 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -25,11 +25,11 @@ class _IdController(Controller): class _OneAttrController(Controller): - foo = AttrR(int) + foo: AttrR[int] class _OtherAttrController(Controller): - bar = AttrR(int) + bar: AttrR[int] def test_controller_api_path_uses_id(): @@ -302,7 +302,7 @@ class names, so ``DEV-1`` and ``DEV_1`` would silently override each other in class _LifecycleController(Controller): """Records lifecycle hook calls for end-to-end assertions.""" - foo = AttrR(int) + foo: AttrR[int] def __init__(self): super().__init__() @@ -324,7 +324,7 @@ async def disconnect(self): class _OtherLifecycleController(_LifecycleController): - bar = AttrR(int) + bar: AttrR[int] @pytest.mark.asyncio diff --git a/tests/test_typed_commands.py b/tests/test_typed_commands.py index c53a78afc..e33aedd75 100644 --- a/tests/test_typed_commands.py +++ b/tests/test_typed_commands.py @@ -29,7 +29,7 @@ class TypedCommandController(Controller): calls: list[tuple] = [] # The GraphQL transport refuses an API with nothing to read - position = AttrR(float) + position: AttrR[float] @command() async def stop(self) -> None: diff --git a/tests/transports/epics/ca/test_gui.py b/tests/transports/epics/ca/test_gui.py index b8586c469..997f60dd6 100644 --- a/tests/transports/epics/ca/test_gui.py +++ b/tests/transports/epics/ca/test_gui.py @@ -193,11 +193,11 @@ def test_get_command_component(): class _A(Controller): - foo = AttrR(int) + foo: AttrR[int] class _B(Controller): - bar = AttrR(int) + bar: AttrR[int] def _api_with_id(cls, name): diff --git a/tests/transports/epics/ca/test_initial_value.py b/tests/transports/epics/ca/test_initial_value.py index 9563e7e55..bdbf0faf8 100644 --- a/tests/transports/epics/ca/test_initial_value.py +++ b/tests/transports/epics/ca/test_initial_value.py @@ -19,32 +19,35 @@ class InitialEnum(enum.Enum): class InitialValuesController(Controller): - int_rw = AttrRW(int, initial_value=4) - float_rw = AttrRW(float, initial_value=3.1) - bool_rw = AttrRW(bool, initial_value=True) - enum_rw = AttrRW(InitialEnum, initial_value=InitialEnum.B) - str_rw = AttrRW(str, initial_value="initial") - waveform_rw = AttrRW( - Array1D[np.int64], - initial_value=np.array(range(10), dtype=np.int64), - shape=(10,), - ) - int_r = AttrR(int, initial_value=5) - float_r = AttrR(float, initial_value=4.1) - bool_r = AttrR(bool, initial_value=False) - enum_r = AttrR(InitialEnum, initial_value=InitialEnum.C) - str_r = AttrR(str, initial_value="initial_r") - waveform_r = AttrR( - Array1D[np.int64], - initial_value=np.array(range(10, 20), dtype=np.int64), - shape=(10,), - ) - int_w = AttrW(int) - float_w = AttrW(float) - bool_w = AttrW(bool) - enum_w = AttrW(InitialEnum) - str_w = AttrW(str) - waveform_w = AttrW(Array1D[np.int64], shape=(10,)) + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.int_rw = AttrRW(int, initial_value=4) + self.float_rw = AttrRW(float, initial_value=3.1) + self.bool_rw = AttrRW(bool, initial_value=True) + self.enum_rw = AttrRW(InitialEnum, initial_value=InitialEnum.B) + self.str_rw = AttrRW(str, initial_value="initial") + self.waveform_rw = AttrRW( + Array1D[np.int64], + initial_value=np.array(range(10), dtype=np.int64), + shape=(10,), + ) + self.int_r = AttrR(int, initial_value=5) + self.float_r = AttrR(float, initial_value=4.1) + self.bool_r = AttrR(bool, initial_value=False) + self.enum_r = AttrR(InitialEnum, initial_value=InitialEnum.C) + self.str_r = AttrR(str, initial_value="initial_r") + self.waveform_r = AttrR( + Array1D[np.int64], + initial_value=np.array(range(10, 20), dtype=np.int64), + shape=(10,), + ) + self.waveform_w = AttrW(Array1D[np.int64], shape=(10,)) + + int_w: AttrW[int] + float_w: AttrW[float] + bool_w: AttrW[bool] + enum_w: AttrW[InitialEnum] + str_w: AttrW[str] @pytest.mark.forked diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index a29958cdd..18a776fe1 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -355,14 +355,17 @@ def test_get_output_record_raises(mocker: MockerFixture): class EpicsController(MyTestController): - read_int = AttrR(int) - read_write_int = AttrRW(int) - read_write_float = AttrRW(float) - read_bool = AttrR(bool) - write_bool = AttrW(bool) - read_string = AttrRW(str) - enum = AttrRW(enum.IntEnum("Enum", {"RED": 0, "GREEN": 1, "BLUE": 2})) - one_d_waveform = AttrRW(Array1D[np.int32], shape=(10,)) + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.enum = AttrRW(enum.IntEnum("Enum", {"RED": 0, "GREEN": 1, "BLUE": 2})) + self.one_d_waveform = AttrRW(Array1D[np.int32], shape=(10,)) + + read_int: AttrR[int] + read_write_int: AttrRW[int] + read_write_float: AttrRW[float] + read_bool: AttrR[bool] + write_bool: AttrW[bool] + read_string: AttrRW[str] @pytest.fixture() @@ -585,9 +588,9 @@ async def do_nothing(): ... class ControllerLongNames(Controller): - attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name = AttrR(int) - attr_rw_with_a_reallyreally_long_name_that_is_too_long_for_rbv = AttrRW(int) - attr_rw_short_name = AttrRW(int) + attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name: AttrR[int] + attr_rw_with_a_reallyreally_long_name_that_is_too_long_for_rbv: AttrRW[int] + attr_rw_short_name: AttrRW[int] command_with_reallyreallyreallyreallyreallyreallyreally_long_name = Command( do_nothing ) diff --git a/tests/transports/epics/pva/test_p4p.py b/tests/transports/epics/pva/test_p4p.py index 56fe9b897..ff63015c8 100644 --- a/tests/transports/epics/pva/test_p4p.py +++ b/tests/transports/epics/pva/test_p4p.py @@ -2,6 +2,7 @@ import enum from datetime import datetime from multiprocessing import Queue +from typing import cast from unittest.mock import ANY from uuid import uuid4 @@ -224,17 +225,20 @@ def make_fastcs(pv_prefix: str, controller: Controller) -> FastCS: def test_read_signal_set(): class SomeController(Controller): - a: AttrRW = AttrRW( - int, - limits=NumericLimits( - control=Limits(high=400_000), alarm=Limits(high=40_000) - ), - ) - b: AttrR = AttrR( - float, - limits=NumericLimits(control=Limits(low=-1), alarm=Limits(low=-0.5)), - precision=2, - ) + def __init__(self) -> None: + super().__init__() + + self.a = AttrRW( + int, + limits=NumericLimits( + control=Limits(high=400_000), alarm=Limits(high=40_000) + ), + ) + self.b = AttrR( + float, + limits=NumericLimits(control=Limits(low=-1), alarm=Limits(low=-0.5)), + precision=2, + ) controller = SomeController() pv_prefix = str(uuid4()) @@ -274,29 +278,31 @@ async def _wait_and_set_attr_r(): def test_pvi_grouping(): class ChildChildController(Controller): - attr_e: AttrRW = AttrRW(int) - attr_f: AttrR = AttrR(str) + attr_e: AttrRW[int] + attr_f: AttrR[str] class ChildController(Controller): - attr_c: AttrW = AttrW(bool, description="Some bool") - attr_d: AttrW = AttrW(str) + attr_d: AttrW[str] + + def __init__(self) -> None: + super().__init__() + self.attr_c = AttrW(bool, description="Some bool") class SomeController(Controller): description = "some controller" - attr_1: AttrRW = AttrRW( - int, - limits=NumericLimits( - control=Limits(high=400_000), alarm=Limits(high=40_000) - ), - ) - attr_1: AttrRW = AttrRW( - float, - limits=NumericLimits(control=Limits(low=-1), alarm=Limits(low=-0.5)), - precision=2, - ) - another_attr_0: AttrRW = AttrRW(int) - another_attr_1000: AttrRW = AttrRW(int) - a_third_attr: AttrW = AttrW(int) + + another_attr_0: AttrRW[int] + another_attr_1000: AttrRW[int] + a_third_attr: AttrW[int] + + def __init__(self) -> None: + super().__init__() + + self.attr_1 = AttrRW( + float, + limits=NumericLimits(control=Limits(low=-1), alarm=Limits(low=-0.5)), + precision=2, + ) controller = SomeController() @@ -433,9 +439,13 @@ class AnEnum(enum.Enum): C = 3 class SomeController(Controller): - some_waveform: AttrRW = AttrRW(Array1D[np.int64], shape=(10, 10)) - some_table: AttrRW = AttrRW(Table, structured_dtype=table_columns) - some_enum: AttrRW = AttrRW(AnEnum) + some_enum: AttrRW[AnEnum] + + def __init__(self) -> None: + super().__init__() + + self.some_waveform = AttrRW(Array1D[np.int64], shape=(10, 10)) + self.some_table = AttrRW(Table, structured_dtype=table_columns) controller = SomeController() pv_prefix = str(uuid4()) @@ -467,7 +477,9 @@ async def _wait_and_set_attrs(): # resulting in only a change in the read back. await asyncio.gather( controller.some_waveform.update(server_set_waveform_value), - controller.some_table.update(server_set_table_value), + # A `Table` is held as a plain structured ndarray; the attribute + # validates the columns at runtime. + controller.some_table.update(cast(Table, server_set_table_value)), controller.some_enum.update(server_set_enum_value), ) diff --git a/tests/transports/epics/test_emission.py b/tests/transports/epics/test_emission.py index e83d0ac3f..c5bc2e095 100644 --- a/tests/transports/epics/test_emission.py +++ b/tests/transports/epics/test_emission.py @@ -23,11 +23,11 @@ class _Alpha(Controller): - foo = AttrR(int) + foo: AttrR[int] class _Beta(Controller): - bar = AttrR(int) + bar: AttrR[int] def _api_with_id(controller_class: type[Controller], name: str): diff --git a/tests/transports/graphQL/test_graphql.py b/tests/transports/graphQL/test_graphql.py index ae618141c..af09d65b4 100644 --- a/tests/transports/graphQL/test_graphql.py +++ b/tests/transports/graphQL/test_graphql.py @@ -16,12 +16,12 @@ class GraphQLController(MyTestController): - read_int = AttrR(int) - read_write_int = AttrRW(int) - read_write_float = AttrRW(float) - read_bool = AttrR(bool) - write_bool = AttrW(bool) - read_string = AttrRW(str) + read_int: AttrR[int] + read_write_int: AttrRW[int] + read_write_float: AttrRW[float] + read_bool: AttrR[bool] + write_bool: AttrW[bool] + read_string: AttrRW[str] _GQL_ID = "device" diff --git a/tests/transports/rest/test_rest.py b/tests/transports/rest/test_rest.py index 8bb7d97cc..3b54e4100 100644 --- a/tests/transports/rest/test_rest.py +++ b/tests/transports/rest/test_rest.py @@ -14,15 +14,18 @@ class RestController(MyTestController): - read_int = AttrR(int) - read_write_int = AttrRW(int) - read_write_float = AttrRW(float) - read_bool = AttrR(bool) - write_bool = AttrW(bool) - read_string = AttrRW(str) - enum = AttrRW(enum.IntEnum("Enum", {"RED": 0, "GREEN": 1, "BLUE": 2})) - one_d_waveform = AttrRW(Array1D[np.int32], shape=(10,)) - two_d_waveform = AttrRW(Array1D[np.int32], shape=(10, 10)) + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.enum = AttrRW(enum.IntEnum("Enum", {"RED": 0, "GREEN": 1, "BLUE": 2})) + self.one_d_waveform = AttrRW(Array1D[np.int32], shape=(10,)) + self.two_d_waveform = AttrRW(Array1D[np.int32], shape=(10, 10)) + + read_int: AttrR[int] + read_write_int: AttrRW[int] + read_write_float: AttrRW[float] + read_bool: AttrR[bool] + write_bool: AttrW[bool] + read_string: AttrRW[str] @pytest.fixture(scope="class") diff --git a/tests/transports/tango/test_dsr.py b/tests/transports/tango/test_dsr.py index abbaa439a..fe9e5a9be 100644 --- a/tests/transports/tango/test_dsr.py +++ b/tests/transports/tango/test_dsr.py @@ -30,15 +30,18 @@ def mock_run_threadsafe_blocking(module_mocker: MockerFixture): class TangoController(MyTestController): - read_int = AttrR(int) - read_write_int = AttrRW(int) - read_write_float = AttrRW(float) - read_bool = AttrR(bool) - write_bool = AttrW(bool) - read_string = AttrRW(str) - enum = AttrRW(enum.IntEnum("Enum", {"RED": 0, "GREEN": 1, "BLUE": 2})) - one_d_waveform = AttrRW(Array1D[np.int32], shape=(10,)) - two_d_waveform = AttrRW(Array1D[np.int32], shape=(10, 10)) + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.enum = AttrRW(enum.IntEnum("Enum", {"RED": 0, "GREEN": 1, "BLUE": 2})) + self.one_d_waveform = AttrRW(Array1D[np.int32], shape=(10,)) + self.two_d_waveform = AttrRW(Array1D[np.int32], shape=(10, 10)) + + read_int: AttrR[int] + read_write_int: AttrRW[int] + read_write_float: AttrRW[float] + read_bool: AttrR[bool] + write_bool: AttrW[bool] + read_string: AttrRW[str] @pytest.fixture(scope="class")