Simplified ID lookup logic (#1716)

* add i09 look up table praiser

* move get_poly to lookuptable

* extracted EnergyMotorLookup base class from i10EnergyMotorLookup

* added new make_phase_tables function

* add test for correct table output

* remove_i09

* add docstring

* add test for helper functions

* add test for skipping

* spacing fix

* Refactor Lookuptable class documentation

Removed outdated docstring from Lookuptable class and updated initialization docstring for I10EnergyMotorLookup class.

* change lookup table schema to snake case

* replace dictionary with basemodel

* add tying

* add gap and phase

* fat finger correction

* Update ID lookup logic to use type checking

* Fix some tests

* Improve models to not use shared defaults

* undo syntax error

* Fixed test to check for success on loading i10 lut

* Simplified lut logic

* Added back generate_lookup_table function

* Fixed all tests but polarisation

* Updated doc strings

* Moved generic test from i10 to test_lookup_table_apple2

* Renamed lut_column_config to lut_config

* Updated more doc strings

* Fix default phase_file name and thus tests

* Use Pol in LookupTable rather than string

* Updated test_convert_csv_to_lookup_overwrite_name_convert_default to use Pol

* Update tests to use Pol rather str value

* Improve EnergyCoverageEntry to have a poly serializer and EnergyCoverage to use float as key

* Add type checking to i10Apple2 phase

* Removed commente out code

* Update i10 id tests to use json files rather than pickle files so they are human readable

* Remove comments

* Fixed poly test

* Fixed test_make_phase_tables_multiple_entries

* Added test_lookup_table_is_serialisable

* Update src/dodal/devices/util/lookup_tables_apple2.py

Co-authored-by: Raymond Fan <[email protected]>

* Fixed formatting

* Improved code coverage

* Simplified ID lookup table logic

* Updated doc strings, tidy tests and variable names

* Made default files a constant

* Update logging messages

* Removed duplicate test logic

* Decoupled path from LookupTableConfig, updated convert_csv_to_lookup to use file_contents again

* Updated doc strings

* Clean up imports

---------

Co-authored-by: Relm-Arrowny <[email protected]>

Author: oliwenmandiamond

src/dodal/beamlines/i10_optics.py

@@ -6,6 +6,8 @@
     idd == id1,    idu == id2.
 """
 
+from pathlib import Path
+
 from daq_config_server.client import ConfigServer
 
 from dodal.common.beamlines.beamline_utils import device_factory
@@ -35,7 +37,9 @@
 from dodal.devices.pgm import PlaneGratingMonochromator
 from dodal.devices.synchrotron import Synchrotron
 from dodal.devices.util.lookup_tables_apple2 import (
-    LookupPath,
+    DEFAULT_GAP_FILE,
+    DEFAULT_PHASE_FILE,
+    EnergyMotorLookup,
     LookupTableConfig,
 )
 from dodal.log import set_beamline as set_log_beamline
@@ -119,13 +123,13 @@ def idd() -> I10Apple2:
 @device_factory()
 def idd_controller() -> I10Apple2Controller:
     """I10 downstream insertion device controller."""
-    return I10Apple2Controller(
-        apple2=idd(),
+    idd_energy_motor_lut = EnergyMotorLookup(
         config_client=I10_CONF_CLIENT,
-        lut_config=LookupTableConfig(
-            source=("Source", "idd"), path=LookupPath.create(LOOK_UPTABLE_DIR)
-        ),
+        lut_config=LookupTableConfig(source=("Source", "idd")),
+        gap_path=Path(LOOK_UPTABLE_DIR, DEFAULT_GAP_FILE),
+        phase_path=Path(LOOK_UPTABLE_DIR, DEFAULT_PHASE_FILE),
     )
+    return I10Apple2Controller(apple2=idd(), energy_motor_lut=idd_energy_motor_lut)
 
 
 @device_factory()
@@ -184,13 +188,13 @@ def idu() -> I10Apple2:
 @device_factory()
 def idu_controller() -> I10Apple2Controller:
     """I10 upstream insertion device controller."""
-    return I10Apple2Controller(
-        apple2=idu(),
+    idu_energy_motor_lut = EnergyMotorLookup(
         config_client=I10_CONF_CLIENT,
-        lut_config=LookupTableConfig(
-            source=("Source", "idu"), path=LookupPath.create(LOOK_UPTABLE_DIR)
-        ),
+        lut_config=LookupTableConfig(source=("Source", "idu")),
+        gap_path=Path(LOOK_UPTABLE_DIR, DEFAULT_GAP_FILE),
+        phase_path=Path(LOOK_UPTABLE_DIR, DEFAULT_PHASE_FILE),
     )
+    return I10Apple2Controller(apple2=idd(), energy_motor_lut=idu_energy_motor_lut)
 
 
 @device_factory()

src/dodal/devices/i10/i10_apple2.py

@@ -2,7 +2,6 @@
 
 import numpy as np
 from bluesky.protocols import Movable
-from daq_config_server.client import ConfigServer
 from ophyd_async.core import (
     AsyncStatus,
     Reference,
@@ -23,11 +22,7 @@
     UndulatorJawPhase,
     UndulatorPhaseAxes,
 )
-from dodal.devices.util.lookup_tables_apple2 import (
-    BaseEnergyMotorLookup,
-    LookupTableConfig,
-    convert_csv_to_lookup,
-)
+from dodal.devices.util.lookup_tables_apple2 import EnergyMotorLookup
 from dodal.log import LOGGER
 
 ROW_PHASE_MOTOR_TOLERANCE = 0.004
@@ -37,35 +32,6 @@
 ALPHA_OFFSET = 180
 
 
-class I10EnergyMotorLookup(BaseEnergyMotorLookup):
-    """
-    Handles lookup tables for I10 Apple2 ID, converting energy and polarisation to gap
-    and phase. Fetches and parses lookup tables from a config server, supports dynamic
-    updates, and validates input.
-    """
-
-    def update_lookuptable(self):
-        """
-        Update lookup tables from files and validate their format.
-        """
-        LOGGER.info("Updating lookup dictionary from file for gap.")
-        gap_csv_file = self.config_client.get_file_contents(
-            self.lut_config.path.gap, reset_cached_result=True
-        )
-        self.lookup_tables.gap = convert_csv_to_lookup(
-            file_contents=gap_csv_file, lut_config=self.lut_config
-        )
-        self.available_pol = list(self.lookup_tables.gap.root.keys())
-
-        LOGGER.info("Updating lookup dictionary from file for phase.")
-        phase_csv_file = self.config_client.get_file_contents(
-            self.lut_config.path.phase, reset_cached_result=True
-        )
-        self.lookup_tables.phase = convert_csv_to_lookup(
-            file_contents=phase_csv_file, lut_config=self.lut_config
-        )
-
-
 class I10Apple2(Apple2[UndulatorPhaseAxes]):
     """I10Apple2 device is an apple2 with extra jaw phase motor."""
 
@@ -102,8 +68,7 @@ class I10Apple2Controller(Apple2Controller[I10Apple2]):
     def __init__(
         self,
         apple2: I10Apple2,
-        config_client: ConfigServer,
-        lut_config: LookupTableConfig,
+        energy_motor_lut: EnergyMotorLookup,
         jaw_phase_limit: float = 12.0,
         jaw_phase_poly_param: list[float] = DEFAULT_JAW_PHASE_POLY_PARAMS,
         angle_threshold_deg=30.0,
@@ -114,10 +79,8 @@ def __init__(
         -----------
         apple2 : I10Apple2
             An I10Apple2 device.
-        config_client : ConfigServer
-            The config server client to fetch the look up table.
-        lut_config:
-            Configuration that defines where the lookup table is and how to read it.
+        energy_motor_lut: EnergyMotorLookup
+            The class that handles the look up table logic for the insertion device.
         jaw_phase_limit : float, optional
             The maximum allowed jaw_phase movement., by default 12.0
         jaw_phase_poly_param : list[float], optional
@@ -128,13 +91,10 @@ def __init__(
             New device name.
         """
 
-        self.lookup_table_client = I10EnergyMotorLookup(
-            lut_config=lut_config,
-            config_client=config_client,
-        )
+        self.energy_motor_lut = energy_motor_lut
         super().__init__(
             apple2=apple2,
-            energy_to_motor_converter=self.lookup_table_client.get_motor_from_energy,
+            energy_to_motor_converter=self.energy_motor_lut.get_motor_from_energy,
             name=name,
         )
 

src/dodal/devices/util/lookup_tables_apple2.py

@@ -27,7 +27,6 @@
 
 """
 
-import abc
 import csv
 import io
 from collections.abc import Generator
@@ -45,34 +44,10 @@
 )
 
 from dodal.devices.apple2_undulator import Pol
+from dodal.log import LOGGER
 
-
-class LookupPath(BaseModel):
-    gap: Path
-    phase: Path
-
-    @classmethod
-    def create(
-        cls,
-        path: str,
-        gap_file: str = "IDEnergy2GapCalibrations.csv",
-        phase_file: str = "IDEnergy2PhaseCalibrations.csv",
-    ) -> "LookupPath":
-        """
-        Factory method to easily create LookupPath using some default file names.
-        Parameters:
-        -----------
-        path:
-            The file path to the lookup tables.
-        gap_file:
-            The gap lookup table file name.
-        phase_file:
-            The phase lookup table file name.
-
-        Returns:
-            LookupPath instance.
-        """
-        return cls(gap=Path(path, gap_file), phase=Path(path, phase_file))
+DEFAULT_GAP_FILE = "IDEnergy2GapCalibrations.csv"
+DEFAULT_PHASE_FILE = "IDEnergy2PhaseCalibrations.csv"
 
 
 DEFAULT_POLY_DEG = [
@@ -90,7 +65,6 @@ def create(
 
 
 class LookupTableConfig(BaseModel):
-    path: LookupPath
     source: tuple[str, str] | None = None
     mode: str = "Mode"
     min_energy: str = "MinEnergy"
@@ -139,7 +113,7 @@ def __init__(self, root: dict[Pol, LookupTableEntries] | None = None):
         super().__init__(root=root or {})
 
 
-class GapPhaseLookupTable(BaseModel):
+class GapPhaseLookupTables(BaseModel):
     gap: LookupTable = Field(default_factory=lambda: LookupTable())
     phase: LookupTable = Field(default_factory=lambda: LookupTable())
 
@@ -155,9 +129,9 @@ def convert_csv_to_lookup(
     Parameters:
     -----------
     file_contents:
-        The CSV file content as a string.
+        The CSV file contents as string.
     lut_config:
-        The configuration that defines how to read the file_contents into a LookupTable
+        The configuration that how to process the file_contents into a LookupTable.
     skip_line_start_with
         Lines beginning with this prefix are skipped (default "#").
 
@@ -315,49 +289,78 @@ def make_phase_tables(
     return lookuptable_phase
 
 
-class BaseEnergyMotorLookup:
+class EnergyMotorLookup:
     """
-    Abstract base for energy->motor lookup.
-
-    Subclasses should implement `update_lookuptable()` to populate `self.lookup_tables`
-    from the configured file sources. After update_lookuptable() has populated the
-    'gap' and 'phase' tables, `get_motor_from_energy()` can be used to compute
-    (gap, phase) for a requested (energy, pol) pair.
+    Handles lookup tables for Apple2 ID, converting energy and polarisation to gap
+    and phase. Fetches and parses lookup tables from a config server, supports dynamic
+    updates, and validates input. If custom logic is required for lookup tables, sub
+    classes should override the _update_gap_lut and _update_phase_lut methods.
+
+    After update_lookuptable() has populated the 'gap' and 'phase' tables,
+    `get_motor_from_energy()` can be used to compute (gap, phase) for a requested
+    (energy, pol) pair.
     """
 
     def __init__(
         self,
         config_client: ConfigServer,
         lut_config: LookupTableConfig,
+        gap_path: Path,
+        phase_path: Path,
     ):
         """Initialise the EnergyMotorLookup class with lookup table headers provided.
 
         Parameters:
         -----------
-        lut_config:
-            The configuration that contains the lookup table file paths and how to read
-            them.
         config_client:
-            The config server client to fetch the look up table.
+            The config server client to fetch the look up table data.
+        lut_config:
+            Configuration that defines how to process file contents into a LookupTable
+        gap_path:
+            File path to the gap lookup table.
+        phase_path:
+            File path to the phase lookup table.
         """
-        self.lookup_tables = GapPhaseLookupTable()
-        self.lut_config = lut_config
+        self.lookup_tables = GapPhaseLookupTables()
         self.config_client = config_client
+        self.lut_config = lut_config
+        self.gap_path = gap_path
+        self.phase_path = phase_path
         self._available_pol = []
 
     @property
-    def available_pol(self) -> list[str | None]:
+    def available_pol(self) -> list[Pol]:
         return self._available_pol
 
     @available_pol.setter
-    def available_pol(self, value: list[str | None]) -> None:
+    def available_pol(self, value: list[Pol]) -> None:
         self._available_pol = value
 
-    @abc.abstractmethod
-    def update_lookuptable(self):
+    def _update_gap_lut(self) -> None:
+        file_contents = self.config_client.get_file_contents(
+            self.gap_path, reset_cached_result=True
+        )
+        self.lookup_tables.gap = convert_csv_to_lookup(
+            file_contents, lut_config=self.lut_config
+        )
+        self.available_pol = list(self.lookup_tables.gap.root.keys())
+
+    def _update_phase_lut(self) -> None:
+        file_contents = self.config_client.get_file_contents(
+            self.phase_path, reset_cached_result=True
+        )
+        self.lookup_tables.phase = convert_csv_to_lookup(
+            file_contents, lut_config=self.lut_config
+        )
+
+    def update_lookuptables(self):
         """
         Update lookup tables from files and validate their format.
         """
+        LOGGER.info("Updating lookup table from file for gap.")
+        self._update_gap_lut()
+        LOGGER.info("Updating lookup table from file for phase.")
+        self._update_phase_lut()
 
     def get_motor_from_energy(self, energy: float, pol: Pol) -> tuple[float, float]:
         """
@@ -376,7 +379,7 @@ def get_motor_from_energy(self, energy: float, pol: Pol) -> tuple[float, float]:
             (gap, phase) motor positions.
         """
         if self.available_pol == []:
-            self.update_lookuptable()
+            self.update_lookuptables()
 
         gap_poly = get_poly(lookup_table=self.lookup_tables.gap, energy=energy, pol=pol)
         phase_poly = get_poly(