Skip to content

Commit

Permalink
(#826) fix or ignore and note typing errors
Browse files Browse the repository at this point in the history
  • Loading branch information
dperl-dls committed Oct 9, 2024
1 parent 49fd0a5 commit 546da96
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 46 deletions.
10 changes: 5 additions & 5 deletions src/dodal/devices/i24/dual_backlight.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)
4 changes: 2 additions & 2 deletions src/dodal/devices/smargon.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions src/dodal/devices/tetramm.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from bluesky.protocols import Hints
from ophyd_async.core import (
DatasetDescriber,
DetectorControl,
DetectorController,
DetectorTrigger,
Device,
PathProvider,
Expand Down Expand Up @@ -80,7 +80,7 @@ def __init__(
super().__init__(name=name)


class TetrammController(DetectorControl):
class TetrammController(DetectorController):
"""Controller for a TetrAMM current monitor
Attributes:
Expand Down
2 changes: 1 addition & 1 deletion src/dodal/devices/util/adjuster_plans.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 10 additions & 7 deletions src/dodal/plans/motor_util_plans.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand 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,
Expand Down
6 changes: 6 additions & 0 deletions src/dodal/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
Any,
TypeGuard,
TypeVar,
runtime_checkable,
)

from bluesky.protocols import (
Expand Down Expand Up @@ -62,6 +63,11 @@
Triggerable,
]


@runtime_checkable
class MovableReadable(Movable, Readable): ...


AnyDevice: TypeAlias = OphydV1Device | OphydV2Device
V1DeviceFactory: TypeAlias = Callable[..., OphydV1Device]
V2DeviceFactory: TypeAlias = Callable[..., OphydV2Device]
Expand Down
4 changes: 2 additions & 2 deletions system_tests/test_aperturescatterguard_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
16 changes: 8 additions & 8 deletions system_tests/test_oav_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/devices/unit_tests/i24/test_pmac.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions tests/devices/unit_tests/test_backlight.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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()


Expand All @@ -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()
40 changes: 26 additions & 14 deletions tests/devices/unit_tests/test_tetramm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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,
)
)


Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
)


Expand All @@ -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,
Expand Down

0 comments on commit 546da96

Please sign in to comment.