Merge pull request #1268 from DiamondLightSource/1218_final_tidy_of_t…

…he_great_ispyb_refactor

Final tidy of the great ispyb refactor

src/hyperion/experiment_plans/robot_load_then_centre_plan.py

@@ -200,6 +200,7 @@ def robot_load_then_centre(
         yield from read_energy(cast(SetEnergyComposite, composite))
     )
 
+    # XXX TODO 1173 remove this - left in to avoid breaking nexus files?
     parameters.hyperion_params.ispyb_params.current_energy_ev = actual_energy_ev
     if not parameters.experiment_params.requested_energy_kev:
         parameters.hyperion_params.detector_params.expected_energy_ev = actual_energy_ev

src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py

@@ -24,12 +24,17 @@
 from hyperion.log import ISPYB_LOGGER
 
 
-def populate_data_collection_group(experiment_type, detector_params, ispyb_params):
+def populate_data_collection_group(
+    experiment_type: str,
+    detector_params: DetectorParams,
+    ispyb_params: IspybParams,
+    sample_barcode: Optional[str] = None,
+):
     dcg_info = DataCollectionGroupInfo(
         visit_string=get_visit_string(ispyb_params, detector_params),
         experiment_type=experiment_type,
         sample_id=ispyb_params.sample_id,
-        sample_barcode=ispyb_params.sample_barcode,
+        sample_barcode=sample_barcode,
     )
     return dcg_info
 
@@ -58,12 +63,8 @@ def populate_remaining_data_collection_info(
     data_collection_info.axis_start = data_collection_info.omega_start
     data_collection_info.focal_spot_size_at_samplex = ispyb_params.focal_spot_size_x
     data_collection_info.focal_spot_size_at_sampley = ispyb_params.focal_spot_size_y
-    data_collection_info.slitgap_vertical = ispyb_params.slit_gap_size_y
-    data_collection_info.slitgap_horizontal = ispyb_params.slit_gap_size_x
     data_collection_info.beamsize_at_samplex = ispyb_params.beam_size_x
     data_collection_info.beamsize_at_sampley = ispyb_params.beam_size_y
-    # Ispyb wants the transmission in a percentage, we use fractions
-    data_collection_info.transmission = ispyb_params.transmission_fraction * 100
     data_collection_info.comments = comment_constructor()
     data_collection_info.detector_distance = detector_params.detector_distance
     data_collection_info.exp_time = detector_params.exposure_time
@@ -74,17 +75,13 @@ def populate_remaining_data_collection_info(
     # planned to be removed later
     data_collection_info.n_passes = 1
     data_collection_info.overlap = 0
-    data_collection_info.flux = ispyb_params.flux
     data_collection_info.start_image_number = 1
     data_collection_info.resolution = ispyb_params.resolution
-    data_collection_info.wavelength = ispyb_params.wavelength_angstroms
     beam_position = detector_params.get_beam_position_mm(
         detector_params.detector_distance
     )
     data_collection_info.xbeam = beam_position[0]
     data_collection_info.ybeam = beam_position[1]
-    data_collection_info.synchrotron_mode = ispyb_params.synchrotron_mode
-    data_collection_info.undulator_gap1 = ispyb_params.undulator_gap
     data_collection_info.start_time = get_current_time_string()
     # temporary file template until nxs filewriting is integrated and we can use
     # that file name

src/hyperion/external_interaction/callbacks/ispyb_callback_base.py

@@ -1,24 +1,34 @@
 from __future__ import annotations
 
 from abc import abstractmethod
+from collections.abc import Sequence
 from typing import TYPE_CHECKING, Any, Callable, Dict, Optional, TypeVar
 
+from hyperion.external_interaction.callbacks.common.ispyb_mapping import (
+    populate_data_collection_group,
+)
 from hyperion.external_interaction.callbacks.plan_reactive_callback import (
     PlanReactiveCallback,
 )
+from hyperion.external_interaction.ispyb.data_model import (
+    DataCollectionInfo,
+    ScanDataInfo,
+)
 from hyperion.external_interaction.ispyb.ispyb_store import (
     IspybIds,
     StoreInIspyb,
 )
 from hyperion.external_interaction.ispyb.ispyb_utils import get_ispyb_config
 from hyperion.log import ISPYB_LOGGER, set_dcgid_tag
 from hyperion.parameters.constants import CONST
+from hyperion.parameters.internal_parameters import InternalParameters
 from hyperion.parameters.plan_specific.gridscan_internal_params import (
     GridscanInternalParameters,
 )
 from hyperion.parameters.plan_specific.rotation_scan_internal_params import (
     RotationInternalParameters,
 )
+from hyperion.utils.utils import convert_eV_to_angstrom
 
 D = TypeVar("D")
 if TYPE_CHECKING:
@@ -36,6 +46,8 @@ def __init__(
         for self.ispyb_ids."""
         ISPYB_LOGGER.debug("Initialising ISPyB callback")
         super().__init__(log=ISPYB_LOGGER, emit=emit)
+        self._event_driven_data_collection_info: Optional[DataCollectionInfo] = None
+        self._sample_barcode: Optional[str] = None
         self.params: GridscanInternalParameters | RotationInternalParameters | None = (
             None
         )
@@ -56,6 +68,8 @@ def __init__(
         self.log = ISPYB_LOGGER
 
     def activity_gated_start(self, doc: RunStart):
+        self._event_driven_data_collection_info = DataCollectionInfo()
+        self._sample_barcode = None
         return self._tag_doc(doc)
 
     def activity_gated_descriptor(self, doc: EventDescriptor):
@@ -77,41 +91,83 @@ def activity_gated_event(self, doc: Event) -> Event:
             )
             return doc
         if event_descriptor.get("name") == CONST.PLAN.ISPYB_HARDWARE_READ:
+            assert self._event_driven_data_collection_info
             ISPYB_LOGGER.info("ISPyB handler received event from read hardware")
-            self.params.hyperion_params.ispyb_params.undulator_gap = doc["data"][
+            self._event_driven_data_collection_info.undulator_gap1 = doc["data"][
                 "undulator_current_gap"
             ]
-            self.params.hyperion_params.ispyb_params.synchrotron_mode = doc["data"][
+            self._event_driven_data_collection_info.synchrotron_mode = doc["data"][
                 "synchrotron-synchrotron_mode"
             ]
-            self.params.hyperion_params.ispyb_params.slit_gap_size_x = doc["data"][
+            self._event_driven_data_collection_info.slitgap_horizontal = doc["data"][
                 "s4_slit_gaps_xgap"
             ]
-            self.params.hyperion_params.ispyb_params.slit_gap_size_y = doc["data"][
+            self._event_driven_data_collection_info.slitgap_vertical = doc["data"][
                 "s4_slit_gaps_ygap"
             ]
-            self.params.hyperion_params.ispyb_params.sample_barcode = doc["data"][
-                "robot-barcode"
-            ]
+            self._sample_barcode = doc["data"]["robot-barcode"]
 
         if event_descriptor.get("name") == CONST.PLAN.ISPYB_TRANSMISSION_FLUX_READ:
-            self.params.hyperion_params.ispyb_params.transmission_fraction = doc[
-                "data"
-            ]["attenuator_actual_transmission"]
-            self.params.hyperion_params.ispyb_params.flux = doc["data"][
+            assert self._event_driven_data_collection_info
+            if doc["data"]["attenuator_actual_transmission"]:
+                # Ispyb wants the transmission in a percentage, we use fractions
+                self._event_driven_data_collection_info.transmission = (
+                    doc["data"]["attenuator_actual_transmission"] * 100
+                )
+                # TODO 1173 Remove this once nexus_utils no longer needs it
+                self.params.hyperion_params.ispyb_params.transmission_fraction = doc[
+                    "data"
+                ]["attenuator_actual_transmission"]
+            self._event_driven_data_collection_info.flux = doc["data"][
                 "flux_flux_reading"
             ]
-            self.params.hyperion_params.ispyb_params.current_energy_ev = (
-                doc["data"]["dcm_energy_in_kev"] * 1000
+            # TODO 1173 Remove this once nexus_utils no longer needs it
+            self.params.hyperion_params.ispyb_params.flux = (
+                self._event_driven_data_collection_info.flux
+            )
+            if doc["data"]["dcm_energy_in_kev"]:
+                energy_ev = doc["data"]["dcm_energy_in_kev"] * 1000
+                self._event_driven_data_collection_info.wavelength = (
+                    convert_eV_to_angstrom(energy_ev)
+                )
+                # TODO 1173 Remove this once nexus_utils no longer needs wavelength_angstroms
+                self.params.hyperion_params.ispyb_params.current_energy_ev = energy_ev
+
+            scan_data_infos = self.populate_info_for_update(
+                self._event_driven_data_collection_info, self.params
             )
-
             ISPYB_LOGGER.info("Updating ispyb entry.")
-            self.ispyb_ids = self.update_deposition(self.params)
+            self.ispyb_ids = self.update_deposition(
+                self.params,
+                scan_data_infos,
+                self._sample_barcode,
+            )
             ISPYB_LOGGER.info(f"Recieved ISPYB IDs: {self.ispyb_ids}")
         return self._tag_doc(doc)
 
+    def update_deposition(
+        self,
+        params,
+        scan_data_infos: Sequence[ScanDataInfo],
+        sample_barcode: Optional[str],
+    ) -> IspybIds:
+        data_collection_group_info = populate_data_collection_group(
+            self.ispyb.experiment_type,
+            params.hyperion_params.detector_params,
+            params.hyperion_params.ispyb_params,
+            sample_barcode,
+        )
+
+        return self.ispyb.update_deposition(
+            self.ispyb_ids, data_collection_group_info, scan_data_infos
+        )
+
     @abstractmethod
-    def update_deposition(self, params) -> IspybIds:
+    def populate_info_for_update(
+        self,
+        event_sourced_data_collection_info: DataCollectionInfo,
+        params: InternalParameters,
+    ) -> Sequence[ScanDataInfo]:
         pass
 
     def activity_gated_stop(self, doc: RunStop) -> RunStop:

src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py

@@ -1,6 +1,8 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Any, Callable
+from collections.abc import Sequence
+from dataclasses import asdict, replace
+from typing import TYPE_CHECKING, Any, Callable, cast
 
 from hyperion.external_interaction.callbacks.common.ispyb_mapping import (
     populate_data_collection_group,
@@ -14,7 +16,11 @@
     construct_comment_for_rotation_scan,
     populate_data_collection_info_for_rotation,
 )
-from hyperion.external_interaction.ispyb.data_model import ExperimentType, ScanDataInfo
+from hyperion.external_interaction.ispyb.data_model import (
+    DataCollectionInfo,
+    ExperimentType,
+    ScanDataInfo,
+)
 from hyperion.external_interaction.ispyb.ispyb_store import (
     IspybIds,
     StoreInIspyb,
@@ -120,29 +126,37 @@ def activity_gated_start(self, doc: RunStart):
             self.uid_to_finalize_on = doc.get("uid")
         return super().activity_gated_start(doc)
 
-    def update_deposition(self, params):
-        dcg_info = populate_data_collection_group(
-            self.ispyb.experiment_type,
-            params.hyperion_params.detector_params,
+    def populate_info_for_update(
+        self, event_sourced_data_collection_info: DataCollectionInfo, params
+    ) -> Sequence[ScanDataInfo]:
+        params = cast(RotationInternalParameters, params)
+        initial_collection_info = populate_data_collection_info_for_rotation(
             params.hyperion_params.ispyb_params,
+            params.hyperion_params.detector_params,
+            params,
         )
-        scan_data_info = ScanDataInfo(
-            data_collection_info=populate_remaining_data_collection_info(
-                construct_comment_for_rotation_scan,
-                self.ispyb_ids.data_collection_group_id,
-                populate_data_collection_info_for_rotation(
-                    params.hyperion_params.ispyb_params,
+        initial_collection_info = replace(
+            initial_collection_info,
+            **{
+                k: v
+                for (k, v) in asdict(event_sourced_data_collection_info).items()
+                if v
+            },
+        )
+        return [
+            ScanDataInfo(
+                data_collection_info=populate_remaining_data_collection_info(
+                    construct_comment_for_rotation_scan,
+                    self.ispyb_ids.data_collection_group_id,
+                    initial_collection_info,
                     params.hyperion_params.detector_params,
-                    params,
+                    params.hyperion_params.ispyb_params,
                 ),
-                params.hyperion_params.detector_params,
-                params.hyperion_params.ispyb_params,
-            ),
-            data_collection_position_info=populate_data_collection_position_info(
-                params.hyperion_params.ispyb_params
-            ),
-        )
-        return self.ispyb.update_deposition(self.ispyb_ids, dcg_info, [scan_data_info])
+                data_collection_position_info=populate_data_collection_position_info(
+                    params.hyperion_params.ispyb_params
+                ),
+            )
+        ]
 
     def activity_gated_event(self, doc: Event):
         doc = super().activity_gated_event(doc)

src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py

@@ -1,7 +1,9 @@
 from __future__ import annotations
 
+from collections.abc import Sequence
+from dataclasses import asdict, replace
 from time import time
-from typing import TYPE_CHECKING, Any, Callable, List, Optional
+from typing import TYPE_CHECKING, Any, Callable, List, Optional, cast
 
 import numpy as np
 from dodal.devices.zocalo.zocalo_results import ZOCALO_READING_PLAN_NAME
@@ -22,7 +24,11 @@
     populate_xz_data_collection_info,
 )
 from hyperion.external_interaction.exceptions import ISPyBDepositionNotMade
-from hyperion.external_interaction.ispyb.data_model import ExperimentType, ScanDataInfo
+from hyperion.external_interaction.ispyb.data_model import (
+    DataCollectionInfo,
+    ExperimentType,
+    ScanDataInfo,
+)
 from hyperion.external_interaction.ispyb.ispyb_store import (
     IspybIds,
     StoreInIspyb,
@@ -163,22 +169,24 @@ def activity_gated_event(self, doc: Event):
 
         return doc
 
-    def update_deposition(self, params):
-        data_collection_group_info = populate_data_collection_group(
-            self.ispyb.experiment_type,
-            params.hyperion_params.detector_params,
-            params.hyperion_params.ispyb_params,
-        )
-
-        scan_data_infos = [self.populate_xy_scan_data_info(params)]
+    def populate_info_for_update(
+        self, event_sourced_data_collection_info: DataCollectionInfo, params
+    ) -> Sequence[ScanDataInfo]:
+        params = cast(GridscanInternalParameters, params)
+        scan_data_infos = [
+            self.populate_xy_scan_data_info(params, event_sourced_data_collection_info)
+        ]
         if self.is_3d_gridscan():
-            scan_data_infos.append(self.populate_xz_scan_data_info(params))
-
-        return self.ispyb.update_deposition(
-            self.ispyb_ids, data_collection_group_info, scan_data_infos
-        )
+            scan_data_infos.append(
+                self.populate_xz_scan_data_info(
+                    params, event_sourced_data_collection_info
+                )
+            )
+        return scan_data_infos
 
-    def populate_xy_scan_data_info(self, params):
+    def populate_xy_scan_data_info(
+        self, params, event_sourced_data_collection_info: DataCollectionInfo
+    ):
         grid_scan_info = GridScanInfo(
             [
                 int(params.hyperion_params.ispyb_params.upper_left[0]),
@@ -195,6 +203,15 @@ def populate_xy_scan_data_info(self, params):
             params.hyperion_params.detector_params,
         )
 
+        xy_data_collection_info = replace(
+            xy_data_collection_info,
+            **{
+                k: v
+                for (k, v) in asdict(event_sourced_data_collection_info).items()
+                if v
+            },
+        )
+
         def comment_constructor():
             return construct_comment_for_gridscan(
                 params, params.hyperion_params.ispyb_params, grid_scan_info
@@ -218,7 +235,9 @@ def comment_constructor():
             ),
         )
 
-    def populate_xz_scan_data_info(self, params):
+    def populate_xz_scan_data_info(
+        self, params, event_sourced_data_collection_info: DataCollectionInfo
+    ):
         xz_grid_scan_info = GridScanInfo(
             [
                 int(params.hyperion_params.ispyb_params.upper_left[0]),
@@ -233,6 +252,14 @@ def populate_xz_scan_data_info(self, params):
             params.hyperion_params.ispyb_params,
             params.hyperion_params.detector_params,
         )
+        xz_data_collection_info = replace(
+            xz_data_collection_info,
+            **{
+                k: v
+                for (k, v) in asdict(event_sourced_data_collection_info).items()
+                if v
+            },
+        )
 
         def xz_comment_constructor():
             return construct_comment_for_gridscan(