From 08268c3b6aeee1ca35363694d9fe6ca4c8fb295b Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 9 Oct 2024 16:08:54 +0100 Subject: [PATCH] (#826) Fix, or ignore and note, typing errors (#830) * (#826) fix or ignore and note typing errors * (#826) update ophyd-async dependency * (#826) put upper bound on ophyd-async for safety * (#826) make backlight conform to Moveable --- pyproject.toml | 10 ++--- src/dodal/devices/backlight.py | 11 ++--- src/dodal/devices/i24/dual_backlight.py | 10 ++--- src/dodal/devices/smargon.py | 4 +- src/dodal/devices/tetramm.py | 4 +- src/dodal/devices/util/adjuster_plans.py | 2 +- src/dodal/plans/motor_util_plans.py | 17 ++++---- src/dodal/utils.py | 7 ++++ .../test_aperturescatterguard_system.py | 4 +- system_tests/test_oav_system.py | 16 ++++---- tests/devices/unit_tests/i24/test_pmac.py | 2 +- tests/devices/unit_tests/test_backlight.py | 8 ++-- tests/devices/unit_tests/test_tetramm.py | 40 ++++++++++++------- 13 files changed, 79 insertions(+), 56 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b65b66a9a2..9202389772 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,7 +15,7 @@ description = "Ophyd devices and other utils that could be used across DLS beaml dependencies = [ "click", "ophyd", - "ophyd-async>=0.6,<0.7", + "ophyd-async>=0.7.0a1,<0.8", "bluesky", "pyepics", "dataclasses-json", @@ -24,10 +24,10 @@ dependencies = [ "requests", "graypy", "pydantic>=2.0", - "opencv-python-headless", # For pin-tip detection. - "aioca", # Required for CA support with ophyd-async. - "p4p", # Required for PVA support with ophyd-async. - "numpy<2.0", # Unpin when https://github.com/bluesky/ophyd-async/issues/387 resolved + "opencv-python-headless", # For pin-tip detection. + "aioca", # Required for CA support with ophyd-async. + "p4p", # Required for PVA support with ophyd-async. + "numpy<2.0", # Unpin when https://github.com/bluesky/ophyd-async/issues/387 resolved "aiofiles", "aiohttp", "redis", diff --git a/src/dodal/devices/backlight.py b/src/dodal/devices/backlight.py index 6cc9029f46..6e5e680b3b 100644 --- a/src/dodal/devices/backlight.py +++ b/src/dodal/devices/backlight.py @@ -1,6 +1,7 @@ from asyncio import sleep from enum import Enum +from bluesky.protocols import Movable from ophyd_async.core import AsyncStatus, StandardReadable from ophyd_async.epics.signal import epics_signal_rw @@ -15,7 +16,7 @@ class BacklightPosition(str, Enum): OUT = "Out" -class Backlight(StandardReadable): +class Backlight(StandardReadable, Movable): """Simple device to trigger the pneumatic in/out.""" TIME_TO_MOVE_S = 1 # Tested using a stopwatch on the beamline 09/2024 @@ -29,7 +30,7 @@ def __init__(self, prefix: str, name: str = "") -> None: super().__init__(name) @AsyncStatus.wrap - async def set(self, position: BacklightPosition): + async def set(self, value: BacklightPosition): """This setter will turn the backlight on when we move it in to the beam and off when we move it out. @@ -38,10 +39,10 @@ async def set(self, position: BacklightPosition): to move completely in/out so we sleep here to simulate this. """ old_position = await self.position.get_value() - await self.position.set(position) - if position == BacklightPosition.OUT: + await self.position.set(value) + if value == BacklightPosition.OUT: await self.power.set(BacklightPower.OFF) else: await self.power.set(BacklightPower.ON) - if old_position != position: + if old_position != value: await sleep(self.TIME_TO_MOVE_S) diff --git a/src/dodal/devices/i24/dual_backlight.py b/src/dodal/devices/i24/dual_backlight.py index 394c058705..ed3deb877f 100644 --- a/src/dodal/devices/i24/dual_backlight.py +++ b/src/dodal/devices/i24/dual_backlight.py @@ -26,8 +26,8 @@ def __init__(self, prefix: str, name: str = "") -> None: super().__init__(name) @AsyncStatus.wrap - async def set(self, position: BacklightPositions): - await self.pos_level.set(position, wait=True) + async def set(self, value: BacklightPositions): + await self.pos_level.set(value, wait=True) class DualBacklight(StandardReadable): @@ -53,9 +53,9 @@ def __init__(self, prefix: str, name: str = "") -> None: super().__init__(name) @AsyncStatus.wrap - async def set(self, position: BacklightPositions): - await self.backlight_position.set(position) - if position == BacklightPositions.OUT: + async def set(self, value: BacklightPositions): + await self.backlight_position.set(value) + if value == BacklightPositions.OUT: await self.backlight_state.set(LEDStatus.OFF, wait=True) else: await self.backlight_state.set(LEDStatus.ON, wait=True) diff --git a/src/dodal/devices/smargon.py b/src/dodal/devices/smargon.py index 50ddc944cf..bffff879b3 100644 --- a/src/dodal/devices/smargon.py +++ b/src/dodal/devices/smargon.py @@ -40,8 +40,8 @@ def __init__(self, name: str = "", prefix: str = ""): super().__init__(name) @AsyncStatus.wrap - async def set(self, pos: StubPosition): - if pos == StubPosition.CURRENT_AS_CENTER: + async def set(self, value: StubPosition): + if value == StubPosition.CURRENT_AS_CENTER: await self.center_at_current_position.set(1) smargon = cast(Smargon, self.parent) await wait_for_value( diff --git a/src/dodal/devices/tetramm.py b/src/dodal/devices/tetramm.py index 20e35e4afd..844e932376 100644 --- a/src/dodal/devices/tetramm.py +++ b/src/dodal/devices/tetramm.py @@ -4,7 +4,7 @@ from bluesky.protocols import Hints from ophyd_async.core import ( DatasetDescriber, - DetectorControl, + DetectorController, DetectorTrigger, Device, PathProvider, @@ -80,7 +80,7 @@ def __init__( super().__init__(name=name) -class TetrammController(DetectorControl): +class TetrammController(DetectorController): """Controller for a TetrAMM current monitor Attributes: diff --git a/src/dodal/devices/util/adjuster_plans.py b/src/dodal/devices/util/adjuster_plans.py index f136f827f2..5cf2da7ddd 100644 --- a/src/dodal/devices/util/adjuster_plans.py +++ b/src/dodal/devices/util/adjuster_plans.py @@ -21,6 +21,6 @@ def lookup_table_adjuster( def adjust(group=None) -> Generator[Msg, None, None]: setpoint = lookup_table(input) LOGGER.info(f"lookup_table_adjuster setting {output_device.name} to {setpoint}") - yield from bps.abs_set(output_device, setpoint, group=group) + yield from bps.abs_set(output_device, setpoint, group=group) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 return adjust diff --git a/src/dodal/plans/motor_util_plans.py b/src/dodal/plans/motor_util_plans.py index 80d0e93611..ee4acd4d08 100644 --- a/src/dodal/plans/motor_util_plans.py +++ b/src/dodal/plans/motor_util_plans.py @@ -1,6 +1,6 @@ import uuid from collections.abc import Generator -from typing import Any, TypeVar +from typing import Any, TypeVar, cast from bluesky import plan_stubs as bps from bluesky.preprocessors import finalize_wrapper, pchain @@ -9,13 +9,14 @@ from ophyd_async.epics.motor import Motor from dodal.common import MsgGenerator +from dodal.utils import MovableReadable -AnyDevice = TypeVar("AnyDevice", bound=Device) +MovableReadableDevice = TypeVar("MovableReadableDevice", bound=MovableReadable) class MoveTooLarge(Exception): def __init__( - self, axis: Device, maximum_move: float, position: float, *args: object + self, axis: MovableReadable, maximum_move: float, position: float, *args: object ) -> None: self.axis = axis self.maximum_move = maximum_move @@ -24,10 +25,10 @@ def __init__( def _check_and_cache_values( - devices_and_positions: dict[AnyDevice, float], + devices_and_positions: dict[MovableReadableDevice, float], smallest_move: float, maximum_move: float, -) -> Generator[Msg, Any, dict[AnyDevice, float]]: +) -> Generator[Msg, Any, dict[MovableReadableDevice, float]]: """Caches the positions of all Motors on specified device if they are within smallest_move of home_position. Throws MoveTooLarge if they are outside maximum_move of the home_position @@ -51,7 +52,9 @@ def home_and_reset_wrapper( wait_for_all: bool = True, ) -> MsgGenerator: home_positions = { - axis: 0.0 for _, axis in device.children() if isinstance(axis, Motor) + cast(MovableReadable, axis): 0.0 + for _, axis in device.children() + if isinstance(axis, Motor) } return move_and_reset_wrapper( plan, home_positions, smallest_move, maximum_move, group, wait_for_all @@ -60,7 +63,7 @@ def home_and_reset_wrapper( def move_and_reset_wrapper( plan: MsgGenerator, - device_and_positions: dict[AnyDevice, float], + device_and_positions: dict[MovableReadable, float], smallest_move: float, maximum_move: float, group: str | None = None, diff --git a/src/dodal/utils.py b/src/dodal/utils.py index e11b8f2ca5..99aa1f7c41 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -13,8 +13,10 @@ from types import ModuleType from typing import ( Any, + Protocol, TypeGuard, TypeVar, + runtime_checkable, ) from bluesky.protocols import ( @@ -62,6 +64,11 @@ Triggerable, ] + +@runtime_checkable +class MovableReadable(Movable, Readable, Protocol): ... + + AnyDevice: TypeAlias = OphydV1Device | OphydV2Device V1DeviceFactory: TypeAlias = Callable[..., OphydV1Device] V2DeviceFactory: TypeAlias = Callable[..., OphydV2Device] diff --git a/system_tests/test_aperturescatterguard_system.py b/system_tests/test_aperturescatterguard_system.py index f9adfe86b3..ba62f6a1e4 100644 --- a/system_tests/test_aperturescatterguard_system.py +++ b/system_tests/test_aperturescatterguard_system.py @@ -154,8 +154,8 @@ def monitor_and_moves(): yield from bps.open_run() yield from bps.monitor(ap_sg.aperture.y.motor_done_move, name="ap_y") yield from bps.monitor(ap_sg.scatterguard.y.motor_done_move, name="sg_y") - yield from bps.mv(ap_sg, pos1) - yield from bps.mv(ap_sg, pos2) + yield from bps.mv(ap_sg, pos1) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 + yield from bps.mv(ap_sg, pos2) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 yield from bps.close_run() RE(monitor_and_moves()) diff --git a/system_tests/test_oav_system.py b/system_tests/test_oav_system.py index f25b147f4d..8768594416 100644 --- a/system_tests/test_oav_system.py +++ b/system_tests/test_oav_system.py @@ -17,14 +17,14 @@ def take_snapshot_with_grid(oav: OAV, snapshot_filename, snapshot_directory): oav.wait_for_connection() - yield from bps.abs_set(oav.grid_snapshot.top_left_x, TEST_GRID_TOP_LEFT_X) - yield from bps.abs_set(oav.grid_snapshot.top_left_y, TEST_GRID_TOP_LEFT_Y) - yield from bps.abs_set(oav.grid_snapshot.box_width, TEST_GRID_BOX_WIDTH) - yield from bps.abs_set(oav.grid_snapshot.num_boxes_x, TEST_GRID_NUM_BOXES_X) - yield from bps.abs_set(oav.grid_snapshot.num_boxes_y, TEST_GRID_NUM_BOXES_Y) - yield from bps.abs_set(oav.grid_snapshot.filename, snapshot_filename) - yield from bps.abs_set(oav.grid_snapshot.directory, snapshot_directory) - yield from bps.trigger(oav.grid_snapshot, wait=True) + yield from bps.abs_set(oav.grid_snapshot.top_left_x, TEST_GRID_TOP_LEFT_X) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 + yield from bps.abs_set(oav.grid_snapshot.top_left_y, TEST_GRID_TOP_LEFT_Y) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 + yield from bps.abs_set(oav.grid_snapshot.box_width, TEST_GRID_BOX_WIDTH) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 + yield from bps.abs_set(oav.grid_snapshot.num_boxes_x, TEST_GRID_NUM_BOXES_X) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 + yield from bps.abs_set(oav.grid_snapshot.num_boxes_y, TEST_GRID_NUM_BOXES_Y) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 + yield from bps.abs_set(oav.grid_snapshot.filename, snapshot_filename) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 + yield from bps.abs_set(oav.grid_snapshot.directory, snapshot_directory) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 + yield from bps.trigger(oav.grid_snapshot, wait=True) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 # We need to find a better way of integrating this, see https://github.com/DiamondLightSource/mx-bluesky/issues/183 diff --git a/tests/devices/unit_tests/i24/test_pmac.py b/tests/devices/unit_tests/i24/test_pmac.py index 9a5090c26b..048370925d 100644 --- a/tests/devices/unit_tests/i24/test_pmac.py +++ b/tests/devices/unit_tests/i24/test_pmac.py @@ -35,7 +35,7 @@ def test_pmac_can_be_created(fake_pmac: PMAC): async def test_pmac_motor_move(fake_pmac: PMAC, RE): pos = (1.0, 0.5) - RE(bps.mv(fake_pmac.x, pos[0], fake_pmac.y, pos[1])) + RE(bps.mv(fake_pmac.x, pos[0], fake_pmac.y, pos[1])) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 assert await fake_pmac.x.user_readback.get_value() == 1.0 assert await fake_pmac.y.user_readback.get_value() == 0.5 diff --git a/tests/devices/unit_tests/test_backlight.py b/tests/devices/unit_tests/test_backlight.py index a8a9472c80..4f5721ce8e 100644 --- a/tests/devices/unit_tests/test_backlight.py +++ b/tests/devices/unit_tests/test_backlight.py @@ -39,7 +39,7 @@ async def test_backlight_can_be_written_and_read_from(fake_backlight: Backlight) async def test_when_backlight_moved_out_it_switches_off( mock_sleep: AsyncMock, fake_backlight: Backlight, RE: RunEngine ): - RE(bps.mv(fake_backlight, BacklightPosition.OUT)) + RE(bps.mv(fake_backlight, BacklightPosition.OUT)) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 assert await fake_backlight.position.get_value() == BacklightPosition.OUT assert await fake_backlight.power.get_value() == BacklightPower.OFF @@ -48,7 +48,7 @@ async def test_when_backlight_moved_out_it_switches_off( async def test_when_backlight_moved_in_it_switches_on( mock_sleep, fake_backlight: Backlight, RE: RunEngine ): - RE(bps.mv(fake_backlight, BacklightPosition.IN)) + RE(bps.mv(fake_backlight, BacklightPosition.IN)) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 assert await fake_backlight.position.get_value() == BacklightPosition.IN assert await fake_backlight.power.get_value() == BacklightPower.ON @@ -58,7 +58,7 @@ async def test_given_backlight_in_when_backlight_moved_in_it_does_not_sleep( mock_sleep: AsyncMock, fake_backlight: Backlight, RE: RunEngine ): set_mock_value(fake_backlight.position, BacklightPosition.IN) - RE(bps.mv(fake_backlight, BacklightPosition.IN)) + RE(bps.mv(fake_backlight, BacklightPosition.IN)) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 mock_sleep.assert_not_awaited() @@ -67,5 +67,5 @@ async def test_given_backlight_out_when_backlight_moved_in_it_sleeps( mock_sleep: AsyncMock, fake_backlight: Backlight, RE: RunEngine ): set_mock_value(fake_backlight.position, BacklightPosition.OUT) - RE(bps.mv(fake_backlight, BacklightPosition.IN)) + RE(bps.mv(fake_backlight, BacklightPosition.IN)) # type: ignore # See: https://github.com/DiamondLightSource/dodal/issues/827 mock_sleep.assert_awaited_once() diff --git a/tests/devices/unit_tests/test_tetramm.py b/tests/devices/unit_tests/test_tetramm.py index 9bf3479f24..31973cf4bb 100644 --- a/tests/devices/unit_tests/test_tetramm.py +++ b/tests/devices/unit_tests/test_tetramm.py @@ -56,7 +56,7 @@ async def tetramm(static_path_provider: PathProvider) -> TetrammDetector: @pytest.fixture def supported_trigger_info() -> TriggerInfo: return TriggerInfo( - number=1, + number_of_triggers=1, trigger=DetectorTrigger.constant_gate, deadtime=1.0, livetime=0.02, @@ -68,7 +68,9 @@ async def test_max_frame_rate_is_calculated_correctly( tetramm_controller: TetrammController, ): await tetramm_controller.prepare( - TriggerInfo(number=1, trigger=DetectorTrigger.edge_trigger, livetime=2.0) + TriggerInfo( + number_of_triggers=1, trigger=DetectorTrigger.edge_trigger, livetime=2.0 + ) ) assert tetramm_controller.minimum_exposure == 0.1 @@ -109,13 +111,17 @@ async def test_min_exposure_is_calculated_correctly( # 100_000 / 17 ~ 5800; 5800 * 0.01 = 58; 58 << tetramm_controller.maximum_readings_per_frame await tetramm_controller.prepare( - TriggerInfo(number=1, trigger=DetectorTrigger.edge_trigger, livetime=0.01) + TriggerInfo( + number_of_triggers=1, trigger=DetectorTrigger.edge_trigger, livetime=0.01 + ) ) assert tetramm_controller.readings_per_frame == int(readings_per_time * 0.01) # 100_000 / 17 ~ 5800; 5800 * 0.2 = 1160; 1160 > tetramm_controller.maximum_readings_per_frame await tetramm_controller.prepare( - TriggerInfo(number=1, trigger=DetectorTrigger.edge_trigger, livetime=0.2) + TriggerInfo( + number_of_triggers=1, trigger=DetectorTrigger.edge_trigger, livetime=0.2 + ) ) assert ( tetramm_controller.readings_per_frame @@ -125,7 +131,9 @@ async def test_min_exposure_is_calculated_correctly( # 100_000 / 17 ~ 5800; 5800 * 0.2 = 1160; 1160 < 1200 tetramm_controller.maximum_readings_per_frame = 1200 await tetramm_controller.prepare( - TriggerInfo(number=1, trigger=DetectorTrigger.edge_trigger, livetime=0.1) + TriggerInfo( + number_of_triggers=1, trigger=DetectorTrigger.edge_trigger, livetime=0.1 + ) ) assert tetramm_controller.readings_per_frame == int(readings_per_time * 0.1) @@ -160,7 +168,11 @@ async def test_set_invalid_exposure_for_number_of_values_per_reading( match="Tetramm exposure time must be at least 5e-05s, asked to set it to 4e-05s", ): await tetramm_controller.prepare( - TriggerInfo(number=0, trigger=DetectorTrigger.edge_trigger, livetime=4e-5) + TriggerInfo( + number_of_triggers=0, + trigger=DetectorTrigger.edge_trigger, + livetime=4e-5, + ) ) @@ -187,7 +199,7 @@ async def test_sample_rate_scales_with_exposure_time( await tetramm.prepare( TriggerInfo( - number=100, + number_of_triggers=100, trigger=DetectorTrigger.edge_trigger, deadtime=2e-5, livetime=exposure, @@ -221,7 +233,7 @@ async def test_arm_raises_value_error_for_invalid_trigger_type( ): await tetramm_controller.prepare( TriggerInfo( - number=0, + number_of_triggers=0, trigger=trigger_type, livetime=VALID_TEST_EXPOSURE_TIME, deadtime=VALID_TEST_DEADTIME, @@ -242,7 +254,7 @@ async def test_arm_sets_signals_correctly_given_valid_inputs( ): await tetramm.prepare( TriggerInfo( - number=0, + number_of_triggers=0, trigger=trigger_type, livetime=VALID_TEST_EXPOSURE_TIME, deadtime=VALID_TEST_DEADTIME, @@ -259,7 +271,7 @@ async def test_disarm_disarms_driver( assert (await tetramm_driver.acquire.get_value()) == 0 await tetramm.prepare( TriggerInfo( - number=0, + number_of_triggers=0, trigger=DetectorTrigger.edge_trigger, livetime=VALID_TEST_EXPOSURE_TIME, deadtime=VALID_TEST_DEADTIME, @@ -284,7 +296,7 @@ async def test_prepare_with_too_low_a_deadtime_raises_error( ): await tetramm.prepare( TriggerInfo( - number=5, + number_of_triggers=5, trigger=DetectorTrigger.edge_trigger, deadtime=1.0 / 100_000.0, livetime=VALID_TEST_EXPOSURE_TIME, @@ -299,7 +311,7 @@ async def test_prepare_arms_tetramm( set_mock_value(tetramm.hdf.file_path_exists, True) await tetramm.prepare( TriggerInfo( - number=5, + number_of_triggers=5, trigger=DetectorTrigger.edge_trigger, deadtime=0.1, livetime=VALID_TEST_EXPOSURE_TIME, @@ -347,7 +359,7 @@ async def test_stage_sets_up_accurate_describe_output( async def test_error_if_armed_without_exposure(tetramm_controller: TetrammController): with pytest.raises(ValueError): await tetramm_controller.prepare( - TriggerInfo(number=10, trigger=DetectorTrigger.internal) + TriggerInfo(number_of_triggers=10, trigger=DetectorTrigger.internal) ) @@ -359,7 +371,7 @@ async def test_pilatus_controller( driver = tetramm.drv await controller.prepare( TriggerInfo( - number=1, + number_of_triggers=1, trigger=DetectorTrigger.constant_gate, livetime=VALID_TEST_EXPOSURE_TIME, deadtime=VALID_TEST_DEADTIME,