From 75062ac11828151a34a01b286a05f6870406c964 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 27 Mar 2026 12:21:46 +0000 Subject: [PATCH 1/6] Fix electron analyser regions not accepting field names --- .../electron_analyser/base/base_region.py | 4 +++- .../specs/test_specs_region.py | 18 +++++++++++++++--- .../vgscienta/test_vgsicenta_region.py | 12 ++++++++++-- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index 62bb4b269f7..a86dbe0d29d 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -4,7 +4,7 @@ from typing import Generic, Self, TypeAlias, TypeVar from ophyd_async.core import StrictEnum, SupersetEnum -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, ConfigDict, Field, model_validator from dodal.devices.electron_analyser.base.base_enums import EnergyMode from dodal.devices.electron_analyser.base.base_util import ( @@ -73,6 +73,8 @@ class AbstractBaseRegion( inherit this to extend functionality. All energy units are assumed to be in eV. """ + model_config = ConfigDict(populate_by_name=True) + name: str = "New_region" enabled: bool = False slices: int = 1 diff --git a/tests/devices/electron_analyser/specs/test_specs_region.py b/tests/devices/electron_analyser/specs/test_specs_region.py index c26a1f27dcd..c36288263a3 100644 --- a/tests/devices/electron_analyser/specs/test_specs_region.py +++ b/tests/devices/electron_analyser/specs/test_specs_region.py @@ -5,7 +5,11 @@ from dodal.devices.beamlines.b07 import LensMode from dodal.devices.beamlines.b07_shared import PsuMode from dodal.devices.electron_analyser.base import EnergyMode -from dodal.devices.electron_analyser.specs import AcquisitionMode, SpecsSequence +from dodal.devices.electron_analyser.specs import ( + AcquisitionMode, + SpecsRegion, + SpecsSequence, +) from dodal.devices.selectable_source import SelectedSource from tests.devices.electron_analyser.helper_util import ( assert_region_has_expected_values, @@ -81,7 +85,7 @@ def expected_region_values() -> list[dict[str, Any]]: ] -def test_sequence_get_expected_enabled_region_names( +def test_load_sequence_using_alias_field_names_has_expected_enabled_region_names( sequence: SpecsSequence[LensMode, PsuMode], expected_enabled_region_names: list[str], ) -> None: @@ -90,10 +94,18 @@ def test_sequence_get_expected_enabled_region_names( assert region.name == expected_enabled_region_names[i] -def test_file_loads_into_class_with_expected_values( +def test_load_sequence_using_alias_field_names_has_expected_values( sequence: SpecsSequence[LensMode, PsuMode], expected_region_values: list[dict[str, Any]], ) -> None: assert len(sequence.regions) == len(expected_region_values) for i, r in enumerate(sequence.regions): assert_region_has_expected_values(r, expected_region_values[i]) + + +def test_region_loads_using_field_names_has_expected_values( + expected_region_values: list[dict[str, Any]], +) -> None: + for expected_region in expected_region_values: + r = SpecsRegion[LensMode, PsuMode].model_validate(expected_region) + assert_region_has_expected_values(r, expected_region) diff --git a/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py b/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py index 0e4fe4f88bb..52684b4dbbe 100644 --- a/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py +++ b/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py @@ -105,7 +105,7 @@ def expected_region_values() -> list[dict[str, Any]]: ] -def test_sequence_get_expected_enabled_region_names( +def test_load_sequence_using_alias_field_names_has_expected_enabled_region_names( sequence: VGScientaSequence[LensMode, PsuMode, PassEnergy], expected_enabled_region_names: list[str], ) -> None: @@ -114,10 +114,18 @@ def test_sequence_get_expected_enabled_region_names( assert region.name == expected_enabled_region_names[i] -def test_file_loads_into_class_with_expected_values( +def test_load_sequence_using_alias_field_names_has_expected_values( sequence: VGScientaSequence[LensMode, PsuMode, PassEnergy], expected_region_values: list[dict[str, Any]], ) -> None: assert len(sequence.regions) == len(expected_region_values) for i, r in enumerate(sequence.regions): assert_region_has_expected_values(r, expected_region_values[i]) + + +def test_region_loads_using_field_names_has_expected_values( + expected_region_values: list[dict[str, Any]], +) -> None: + for expected_region in expected_region_values: + r = VGScientaRegion[LensMode, PassEnergy].model_validate(expected_region) + assert_region_has_expected_values(r, expected_region) From 82558d68b73fb88e0f3ee28431fffc1c706608e4 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 27 Mar 2026 13:03:32 +0000 Subject: [PATCH 2/6] Remove unused value _id --- .../devices/electron_analyser/vgscienta/vgscienta_region.py | 2 -- .../electron_analyser/vgscienta/test_vgsicenta_region.py | 3 --- 2 files changed, 5 deletions(-) diff --git a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_region.py b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_region.py index 151b2ef5698..d714949da9d 100644 --- a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_region.py +++ b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_region.py @@ -1,4 +1,3 @@ -import uuid from typing import Generic, TypeVar from ophyd_async.core import StrictEnum @@ -32,7 +31,6 @@ class VGScientaRegion( acquire_time: float = Field(default=1.0, alias="step_time") energy_step: float = Field(default=200.0) # Specific to this class - id: str = Field(default=str(uuid.uuid4()), alias="region_id") total_steps: float = 13.0 total_time: float = 13.0 min_x: int = Field(alias="first_x_channel", default=1) diff --git a/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py b/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py index 52684b4dbbe..7b31933ec44 100644 --- a/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py +++ b/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py @@ -33,7 +33,6 @@ def expected_region_values() -> list[dict[str, Any]]: { "name": "New_Region", "enabled": True, - "id": "_aQOmgPsmEe6w2YUF3bV-LA", "lens_mode": LensMode.ANGULAR56, "pass_energy": PassEnergy.E5, "slices": 1, @@ -57,7 +56,6 @@ def expected_region_values() -> list[dict[str, Any]]: { "name": "New_Region1", "enabled": False, - "id": "_aQOmgPsmEe6w2YUF3GV-LL", "lens_mode": LensMode.ANGULAR45, "pass_energy": PassEnergy.E10, "slices": 10, @@ -81,7 +79,6 @@ def expected_region_values() -> list[dict[str, Any]]: { "name": "New_Region2", "enabled": True, - "id": "_NAVc8ExAEfCC6ZXV-LTq8A", "lens_mode": LensMode.ANGULAR45VUV, "pass_energy": PassEnergy.E20, "slices": 5, From 063bdabe6a4f7889b7b6288bf11ca2b6f0be0d8a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 27 Mar 2026 13:04:03 +0000 Subject: [PATCH 3/6] Correct test file name --- .../{test_vgsicenta_region.py => test_vgscienta_region.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/devices/electron_analyser/vgscienta/{test_vgsicenta_region.py => test_vgscienta_region.py} (100%) diff --git a/tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py b/tests/devices/electron_analyser/vgscienta/test_vgscienta_region.py similarity index 100% rename from tests/devices/electron_analyser/vgscienta/test_vgsicenta_region.py rename to tests/devices/electron_analyser/vgscienta/test_vgscienta_region.py From a7bede99f54019488f05995a52f6c4886a46b000 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 27 Mar 2026 16:35:01 +0000 Subject: [PATCH 4/6] Update base class names to remoce abstract as they are not abstract --- .../electron_analyser/base/__init__.py | 16 ++++----- .../electron_analyser/base/base_controller.py | 6 ++-- .../electron_analyser/base/base_detector.py | 36 ++++++++----------- .../electron_analyser/base/base_driver_io.py | 10 +++--- .../electron_analyser/base/base_region.py | 20 +++++------ .../electron_analyser/specs/specs_region.py | 8 ++--- .../vgscienta/vgscienta_region.py | 8 ++--- .../base/test_base_controller.py | 22 ++++++------ .../base/test_base_region.py | 12 +++---- tests/devices/electron_analyser/conftest.py | 8 ++--- .../helper_util/assert_func.py | 4 +-- 11 files changed, 71 insertions(+), 79 deletions(-) diff --git a/src/dodal/devices/electron_analyser/base/__init__.py b/src/dodal/devices/electron_analyser/base/__init__.py index a06633ee053..cab7f89495a 100644 --- a/src/dodal/devices/electron_analyser/base/__init__.py +++ b/src/dodal/devices/electron_analyser/base/__init__.py @@ -17,13 +17,13 @@ ) from .base_enums import EnergyMode from .base_region import ( - AbstractBaseRegion, - AbstractBaseSequence, + BaseRegion, + BaseSequence, GenericRegion, GenericSequence, - TAbstractBaseRegion, - TAbstractBaseSequence, TAcquisitionMode, + TBaseRegion, + TBaseSequence, TLensMode, ) from .base_util import to_binding_energy, to_kinetic_energy @@ -42,12 +42,12 @@ "GenericAnalyserDriverIO", "TAbstractAnalyserDriverIO", "EnergyMode", - "AbstractBaseRegion", - "AbstractBaseSequence", + "BaseRegion", + "BaseSequence", "GenericRegion", "GenericSequence", - "TAbstractBaseRegion", - "TAbstractBaseSequence", + "TBaseRegion", + "TBaseSequence", "TAcquisitionMode", "TLensMode", "to_binding_energy", diff --git a/src/dodal/devices/electron_analyser/base/base_controller.py b/src/dodal/devices/electron_analyser/base/base_controller.py index 733e608c6f9..70ef253a7e7 100644 --- a/src/dodal/devices/electron_analyser/base/base_controller.py +++ b/src/dodal/devices/electron_analyser/base/base_controller.py @@ -10,7 +10,7 @@ ) from dodal.devices.electron_analyser.base.base_region import ( GenericRegion, - TAbstractBaseRegion, + TBaseRegion, ) from dodal.devices.electron_analyser.base.energy_sources import AbstractEnergySource from dodal.devices.fast_shutter import GenericFastShutter @@ -19,7 +19,7 @@ class ElectronAnalyserController( ConstantDeadTimeController[TAbstractAnalyserDriverIO], - Generic[TAbstractAnalyserDriverIO, TAbstractBaseRegion], + Generic[TAbstractAnalyserDriverIO, TBaseRegion], ): """Specialised controller for the electron analysers to provide additional setup logic such as selecting the energy source to use from requested region and giving @@ -50,7 +50,7 @@ def __init__( self.source_selector = source_selector super().__init__(driver, deadtime, image_mode) - async def setup_with_region(self, region: TAbstractBaseRegion) -> None: + async def setup_with_region(self, region: TBaseRegion) -> None: """Logic to set the driver with a region.""" if self.source_selector is not None: await self.source_selector.set(region.excitation_energy_source) diff --git a/src/dodal/devices/electron_analyser/base/base_detector.py b/src/dodal/devices/electron_analyser/base/base_detector.py index aed52fe418b..08506e9bba1 100644 --- a/src/dodal/devices/electron_analyser/base/base_detector.py +++ b/src/dodal/devices/electron_analyser/base/base_detector.py @@ -19,7 +19,7 @@ ) from dodal.devices.electron_analyser.base.base_region import ( GenericRegion, - TAbstractBaseRegion, + TBaseRegion, ) @@ -28,7 +28,7 @@ class BaseElectronAnalyserDetector( Triggerable, AsyncReadable, AsyncConfigurable, - Generic[TAbstractAnalyserDriverIO, TAbstractBaseRegion], + Generic[TAbstractAnalyserDriverIO, TBaseRegion], ): """Detector for data acquisition of electron analyser. Can only acquire using settings already configured for the device. @@ -40,16 +40,14 @@ class BaseElectronAnalyserDetector( def __init__( self, - controller: ElectronAnalyserController[ - TAbstractAnalyserDriverIO, TAbstractBaseRegion - ], + controller: ElectronAnalyserController[TAbstractAnalyserDriverIO, TBaseRegion], name: str = "", ): self._controller = controller super().__init__(name) @AsyncStatus.wrap - async def set(self, region: TAbstractBaseRegion) -> None: + async def set(self, region: TBaseRegion) -> None: await self._controller.setup_with_region(region) @AsyncStatus.wrap @@ -83,8 +81,8 @@ async def describe_configuration(self) -> dict[str, DataKey]: class ElectronAnalyserRegionDetector( - BaseElectronAnalyserDetector[TAbstractAnalyserDriverIO, TAbstractBaseRegion], - Generic[TAbstractAnalyserDriverIO, TAbstractBaseRegion], + BaseElectronAnalyserDetector[TAbstractAnalyserDriverIO, TBaseRegion], + Generic[TAbstractAnalyserDriverIO, TBaseRegion], ): """Extends electron analyser detector to configure specific region settings before data acquisition. It is designed to only exist inside a plan. @@ -92,10 +90,8 @@ class ElectronAnalyserRegionDetector( def __init__( self, - controller: ElectronAnalyserController[ - TAbstractAnalyserDriverIO, TAbstractBaseRegion - ], - region: TAbstractBaseRegion, + controller: ElectronAnalyserController[TAbstractAnalyserDriverIO, TBaseRegion], + region: TBaseRegion, name: str = "", ): self.region = region @@ -121,9 +117,9 @@ async def trigger(self) -> None: class ElectronAnalyserDetector( - BaseElectronAnalyserDetector[TAbstractAnalyserDriverIO, TAbstractBaseRegion], + BaseElectronAnalyserDetector[TAbstractAnalyserDriverIO, TBaseRegion], Stageable, - Generic[TAbstractAnalyserDriverIO, TAbstractBaseRegion], + Generic[TAbstractAnalyserDriverIO, TBaseRegion], ): """Electron analyser detector with the additional functionality to load a sequence file and create a list of temporary ElectronAnalyserRegionDetector objects. These @@ -150,10 +146,8 @@ async def unstage(self) -> None: await self._controller.disarm() def create_region_detector_list( - self, regions: list[TAbstractBaseRegion] - ) -> list[ - ElectronAnalyserRegionDetector[TAbstractAnalyserDriverIO, TAbstractBaseRegion] - ]: + self, regions: list[TBaseRegion] + ) -> list[ElectronAnalyserRegionDetector[TAbstractAnalyserDriverIO, TBaseRegion]]: """This method can hopefully be dropped when this is merged and released. https://github.com/bluesky/bluesky/pull/1978. @@ -168,9 +162,9 @@ def create_region_detector_list( the sequence file. """ return [ - ElectronAnalyserRegionDetector[ - TAbstractAnalyserDriverIO, TAbstractBaseRegion - ](self._controller, r, self.name + "_" + r.name) + ElectronAnalyserRegionDetector[TAbstractAnalyserDriverIO, TBaseRegion]( + self._controller, r, self.name + "_" + r.name + ) for r in regions ] diff --git a/src/dodal/devices/electron_analyser/base/base_driver_io.py b/src/dodal/devices/electron_analyser/base/base_driver_io.py index 8bc55e7ddf9..c85d384c884 100644 --- a/src/dodal/devices/electron_analyser/base/base_driver_io.py +++ b/src/dodal/devices/electron_analyser/base/base_driver_io.py @@ -23,8 +23,8 @@ AnyLensMode, AnyPassEnergy, GenericRegion, - TAbstractBaseRegion, TAcquisitionMode, + TBaseRegion, TLensMode, TPassEnergy, ) @@ -40,8 +40,8 @@ class AbstractAnalyserDriverIO( ABC, StandardReadable, ADBaseIO, - Movable[TAbstractBaseRegion], - Generic[TAbstractBaseRegion, TAcquisitionMode, TLensMode, TPsuMode, TPassEnergy], + Movable[TBaseRegion], + Generic[TBaseRegion, TAcquisitionMode, TLensMode, TPsuMode, TPassEnergy], ): """Driver device that defines signals and readables that should be common to all electron analysers. Implementations of electron analyser devices should inherit @@ -137,12 +137,12 @@ def __init__( @abstractmethod @AsyncStatus.wrap - async def set(self, epics_region: TAbstractBaseRegion): + async def set(self, epics_region: TBaseRegion): """Move a group of signals defined in a region. Each implementation of this class is responsible for implementing this method correctly. Args: - epics_region (TAbstractBaseRegion): Contains the parameters to setup the + epics_region (TBaseRegion): Contains the parameters to setup the driver for a scan. """ diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index a86dbe0d29d..8ae56709ff0 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -64,7 +64,7 @@ def energy_mode_validation(data: dict) -> dict: return data -class AbstractBaseRegion( +class BaseRegion( ABC, JavaToPythonModel, Generic[TAcquisitionMode, TLensMode, TPassEnergy], @@ -168,14 +168,14 @@ def before_validation(cls, data: dict) -> dict: return energy_mode_validation(data) -GenericRegion = AbstractBaseRegion[AnyAcqMode, AnyLensMode, AnyPassEnergy] -TAbstractBaseRegion = TypeVar("TAbstractBaseRegion", bound=AbstractBaseRegion) +GenericRegion = BaseRegion[AnyAcqMode, AnyLensMode, AnyPassEnergy] +TBaseRegion = TypeVar("TBaseRegion", bound=BaseRegion) -class AbstractBaseSequence( +class BaseSequence( ABC, JavaToPythonModel, - Generic[TAbstractBaseRegion], + Generic[TBaseRegion], ): """Generic sequence model that holds the list of region data. Specialised sequence models should inherit this to extend functionality and define type of region to @@ -183,9 +183,9 @@ class AbstractBaseSequence( """ version: float = 0.1 # If file format changes within prod, increment this number! - regions: list[TAbstractBaseRegion] = Field(default_factory=lambda: []) + regions: list[TBaseRegion] = Field(default_factory=lambda: []) - def get_enabled_regions(self) -> list[TAbstractBaseRegion]: + def get_enabled_regions(self) -> list[TBaseRegion]: return [r for r in self.regions if r.enabled] def get_region_names(self) -> list[str]: @@ -194,9 +194,9 @@ def get_region_names(self) -> list[str]: def get_enabled_region_names(self) -> list[str]: return [r.name for r in self.get_enabled_regions()] - def get_region_by_name(self, name: str) -> TAbstractBaseRegion | None: + def get_region_by_name(self, name: str) -> TBaseRegion | None: return next((region for region in self.regions if region.name == name), None) -GenericSequence = AbstractBaseSequence[GenericRegion] -TAbstractBaseSequence = TypeVar("TAbstractBaseSequence", bound=AbstractBaseSequence) +GenericSequence = BaseSequence[GenericRegion] +TBaseSequence = TypeVar("TBaseSequence", bound=BaseSequence) diff --git a/src/dodal/devices/electron_analyser/specs/specs_region.py b/src/dodal/devices/electron_analyser/specs/specs_region.py index 6654fc18881..cfb6bc426fc 100644 --- a/src/dodal/devices/electron_analyser/specs/specs_region.py +++ b/src/dodal/devices/electron_analyser/specs/specs_region.py @@ -3,8 +3,8 @@ from pydantic import Field from dodal.devices.electron_analyser.base.base_region import ( - AbstractBaseRegion, - AbstractBaseSequence, + BaseRegion, + BaseSequence, TLensMode, TPsuMode, ) @@ -12,7 +12,7 @@ class SpecsRegion( - AbstractBaseRegion[AcquisitionMode, TLensMode, float], + BaseRegion[AcquisitionMode, TLensMode, float], Generic[TLensMode, TPsuMode], ): # Override base class with defaults @@ -32,6 +32,6 @@ class SpecsRegion( class SpecsSequence( - AbstractBaseSequence[SpecsRegion[TLensMode, TPsuMode]], Generic[TLensMode, TPsuMode] + BaseSequence[SpecsRegion[TLensMode, TPsuMode]], Generic[TLensMode, TPsuMode] ): regions: list[SpecsRegion[TLensMode, TPsuMode]] = Field(default_factory=lambda: []) diff --git a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_region.py b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_region.py index d714949da9d..556da7bfa78 100644 --- a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_region.py +++ b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_region.py @@ -4,8 +4,8 @@ from pydantic import Field, field_validator from dodal.devices.electron_analyser.base.base_region import ( - AbstractBaseRegion, - AbstractBaseSequence, + BaseRegion, + BaseSequence, TLensMode, TPsuMode, ) @@ -18,7 +18,7 @@ class VGScientaRegion( - AbstractBaseRegion[AcquisitionMode, TLensMode, TPassEnergyEnum], + BaseRegion[AcquisitionMode, TLensMode, TPassEnergyEnum], Generic[TLensMode, TPassEnergyEnum], ): # Override defaults of base region class @@ -56,7 +56,7 @@ def validate_pass_energy(cls, val): class VGScientaSequence( - AbstractBaseSequence[VGScientaRegion[TLensMode, TPassEnergyEnum]], + BaseSequence[VGScientaRegion[TLensMode, TPassEnergyEnum]], Generic[TLensMode, TPsuMode, TPassEnergyEnum], ): psu_mode: TPsuMode = Field(alias="element_set") diff --git a/tests/devices/electron_analyser/base/test_base_controller.py b/tests/devices/electron_analyser/base/test_base_controller.py index 94986943714..98a7de3d585 100644 --- a/tests/devices/electron_analyser/base/test_base_controller.py +++ b/tests/devices/electron_analyser/base/test_base_controller.py @@ -5,7 +5,7 @@ from dodal.devices.electron_analyser.base import ( AbstractAnalyserDriverIO, - AbstractBaseRegion, + BaseRegion, ElectronAnalyserController, GenericElectronAnalyserController, GenericElectronAnalyserDetector, @@ -31,7 +31,7 @@ def analyser_controller( async def test_controller_prepare_sets_excitation_energy( analyser_controller: ElectronAnalyserController[ - AbstractAnalyserDriverIO, AbstractBaseRegion + AbstractAnalyserDriverIO, BaseRegion ], ) -> None: excitation_energy = await analyser_controller.energy_source.energy.get_value() @@ -44,18 +44,18 @@ async def test_controller_prepare_sets_excitation_energy( @pytest.mark.parametrize("region", TEST_SEQUENCE_REGION_NAMES, indirect=True) async def test_controller_setup_with_region_sets_region_for_epics_and_sets_driver( analyser_controller: ElectronAnalyserController[ - AbstractAnalyserDriverIO, AbstractBaseRegion + AbstractAnalyserDriverIO, BaseRegion ], - region: AbstractBaseRegion, + region: BaseRegion, ) -> None: sim_driver = analyser_controller.driver sim_driver.set = AsyncMock() # Patch switch_energy_mode so we can check on calls, but still run the real function with patch.object( - AbstractBaseRegion, + BaseRegion, "prepare_for_epics", - side_effect=AbstractBaseRegion.prepare_for_epics, # run the real method + side_effect=BaseRegion.prepare_for_epics, # run the real method autospec=True, ) as mock_prepare_for_epics: await analyser_controller.setup_with_region(region) @@ -80,9 +80,9 @@ async def test_controller_setup_with_region_sets_region_for_epics_and_sets_drive @pytest.mark.parametrize("region", TEST_SEQUENCE_REGION_NAMES, indirect=True) async def test_controller_setup_with_region_moves_selected_source_if_not_none( analyser_controller: ElectronAnalyserController[ - AbstractAnalyserDriverIO, AbstractBaseRegion + AbstractAnalyserDriverIO, BaseRegion ], - region: AbstractBaseRegion, + region: BaseRegion, ) -> None: source_selector = analyser_controller.source_selector @@ -96,9 +96,9 @@ async def test_controller_setup_with_region_moves_selected_source_if_not_none( @pytest.mark.parametrize("region", TEST_SEQUENCE_REGION_NAMES, indirect=True) async def test_controller_setup_with_region_moves_shutter_if_not_none( analyser_controller: ElectronAnalyserController[ - AbstractAnalyserDriverIO, AbstractBaseRegion + AbstractAnalyserDriverIO, BaseRegion ], - region: AbstractBaseRegion, + region: BaseRegion, ) -> None: shutter = analyser_controller.shutter if shutter is not None: @@ -110,7 +110,7 @@ async def test_controller_setup_with_region_moves_shutter_if_not_none( async def test_controller_prepare_moves_shutters_if_not_none( analyser_controller: ElectronAnalyserController[ - AbstractAnalyserDriverIO, AbstractBaseRegion + AbstractAnalyserDriverIO, BaseRegion ], ) -> None: shutter = analyser_controller.shutter diff --git a/tests/devices/electron_analyser/base/test_base_region.py b/tests/devices/electron_analyser/base/test_base_region.py index 9d53f4a5e59..bcf7b595d98 100644 --- a/tests/devices/electron_analyser/base/test_base_region.py +++ b/tests/devices/electron_analyser/base/test_base_region.py @@ -4,11 +4,11 @@ from dodal.devices.beamlines import b07, b07_shared, i09 from dodal.devices.electron_analyser.base import ( - AbstractBaseRegion, + BaseRegion, EnergyMode, GenericRegion, GenericSequence, - TAbstractBaseRegion, + TBaseRegion, to_binding_energy, to_kinetic_energy, ) @@ -34,9 +34,7 @@ def sequence(request: pytest.FixtureRequest) -> GenericSequence: @pytest.fixture -def expected_region_class( - sequence: GenericSequence, -) -> type[AbstractBaseRegion]: +def expected_region_class(sequence: GenericSequence) -> type[BaseRegion]: if isinstance(sequence, SpecsSequence): return SpecsRegion[b07.LensMode, b07_shared.PsuMode] elif isinstance(sequence, VGScientaSequence): @@ -59,7 +57,7 @@ def test_sequence_get_expected_region_from_name( def test_sequence_get_expected_region_type( sequence: GenericSequence, - expected_region_class: type[TAbstractBaseRegion], + expected_region_class: type[TBaseRegion], ) -> None: regions = sequence.regions enabled_regions = sequence.get_enabled_regions() @@ -139,7 +137,7 @@ def test_region_prepare_for_epics(region: GenericRegion, copy: bool) -> None: # Patch switch_energy_mode so we can spy on if it was called while also returning # true function return value with patch.object( - AbstractBaseRegion, "switch_energy_mode", wraps=region.switch_energy_mode + BaseRegion, "switch_energy_mode", wraps=region.switch_energy_mode ) as mock_switch_energy_mode: excitation_energy = 500 original_energy_mode = region.energy_mode diff --git a/tests/devices/electron_analyser/conftest.py b/tests/devices/electron_analyser/conftest.py index 912c26ff457..3d4e8c66669 100644 --- a/tests/devices/electron_analyser/conftest.py +++ b/tests/devices/electron_analyser/conftest.py @@ -11,8 +11,8 @@ StationaryCrystal, ) from dodal.devices.electron_analyser.base import ( - AbstractBaseRegion, - AbstractBaseSequence, + BaseRegion, + BaseSequence, DualEnergySource, EnergySource, ) @@ -138,8 +138,8 @@ async def ew4000( @pytest.fixture def region( request: pytest.FixtureRequest, - sequence: AbstractBaseSequence[AbstractBaseRegion], -) -> AbstractBaseRegion: + sequence: BaseSequence[BaseRegion], +) -> BaseRegion: region = sequence.get_region_by_name(request.param) if region is None: raise ValueError("Region " + request.param + " is not found.") diff --git a/tests/devices/electron_analyser/helper_util/assert_func.py b/tests/devices/electron_analyser/helper_util/assert_func.py index 4e20df0bcfe..933cd92a6e6 100644 --- a/tests/devices/electron_analyser/helper_util/assert_func.py +++ b/tests/devices/electron_analyser/helper_util/assert_func.py @@ -2,11 +2,11 @@ from deepdiff import DeepDiff -from dodal.devices.electron_analyser.base import AbstractBaseRegion +from dodal.devices.electron_analyser.base import BaseRegion def assert_region_has_expected_values( - r: AbstractBaseRegion, expected_region_values: dict[str, Any] + r: BaseRegion, expected_region_values: dict[str, Any] ) -> None: actual_values = r.__dict__ diff = DeepDiff(expected_region_values, actual_values) From e58616fb81dbb606e6bc691da6407f969e15cbca Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 27 Mar 2026 16:35:47 +0000 Subject: [PATCH 5/6] Remove unused version number --- src/dodal/devices/electron_analyser/base/base_region.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index 8ae56709ff0..357f9f0e505 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -182,7 +182,6 @@ class BaseSequence( hold. """ - version: float = 0.1 # If file format changes within prod, increment this number! regions: list[TBaseRegion] = Field(default_factory=lambda: []) def get_enabled_regions(self) -> list[TBaseRegion]: From 4df2195fd5058a5f64297d563f09e0ae3dd2f618 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 27 Mar 2026 16:46:53 +0000 Subject: [PATCH 6/6] Apply feedback to reduce code --- tests/devices/electron_analyser/specs/test_specs_region.py | 5 ++--- .../electron_analyser/vgscienta/test_vgscienta_region.py | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/devices/electron_analyser/specs/test_specs_region.py b/tests/devices/electron_analyser/specs/test_specs_region.py index c36288263a3..5151ce141ff 100644 --- a/tests/devices/electron_analyser/specs/test_specs_region.py +++ b/tests/devices/electron_analyser/specs/test_specs_region.py @@ -98,9 +98,8 @@ def test_load_sequence_using_alias_field_names_has_expected_values( sequence: SpecsSequence[LensMode, PsuMode], expected_region_values: list[dict[str, Any]], ) -> None: - assert len(sequence.regions) == len(expected_region_values) - for i, r in enumerate(sequence.regions): - assert_region_has_expected_values(r, expected_region_values[i]) + for i, r in zip(sequence.regions, expected_region_values, strict=True): + assert_region_has_expected_values(i, r) def test_region_loads_using_field_names_has_expected_values( diff --git a/tests/devices/electron_analyser/vgscienta/test_vgscienta_region.py b/tests/devices/electron_analyser/vgscienta/test_vgscienta_region.py index 7b31933ec44..9933b9a0919 100644 --- a/tests/devices/electron_analyser/vgscienta/test_vgscienta_region.py +++ b/tests/devices/electron_analyser/vgscienta/test_vgscienta_region.py @@ -115,9 +115,8 @@ def test_load_sequence_using_alias_field_names_has_expected_values( sequence: VGScientaSequence[LensMode, PsuMode, PassEnergy], expected_region_values: list[dict[str, Any]], ) -> None: - assert len(sequence.regions) == len(expected_region_values) - for i, r in enumerate(sequence.regions): - assert_region_has_expected_values(r, expected_region_values[i]) + for i, r in zip(sequence.regions, expected_region_values, strict=True): + assert_region_has_expected_values(i, r) def test_region_loads_using_field_names_has_expected_values(