Merge pull request DiamondLightSource/hyperion#1340 from DiamondLight…

…Source/1293_remove_barcode 1293 remove barcode
olliesilvester · Apr 29, 2024 · 3daf796 · 3daf796
2 parents d79c7c8 + 0a0ceee
commit 3daf796
Show file tree

Hide file tree

Showing 15 changed files with 29 additions and 150 deletions.
diff --git a/src/hyperion/device_setup_plans/read_hardware_for_setup.py b/src/hyperion/device_setup_plans/read_hardware_for_setup.py
@@ -31,7 +31,6 @@ def read_hardware_for_ispyb_pre_collection(
     yield from bps.read(s4_slit_gaps.xgap)
     yield from bps.read(s4_slit_gaps.ygap)
     yield from bps.read(aperture_scatterguard)
-    yield from bps.read(robot.barcode)
     yield from bps.save()
 
 

diff --git a/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py b/src/hyperion/external_interaction/callbacks/common/ispyb_mapping.py
@@ -23,16 +23,12 @@
 
 
 def populate_data_collection_group(
-    experiment_type: str,
-    detector_params: DetectorParams,
-    ispyb_params: IspybParams,
-    sample_barcode: Optional[str] = None,
+    experiment_type: str, detector_params: DetectorParams, ispyb_params: IspybParams
 ):
     dcg_info = DataCollectionGroupInfo(
         visit_string=get_visit_string(ispyb_params, detector_params),
         experiment_type=experiment_type,
         sample_id=ispyb_params.sample_id,
-        sample_barcode=sample_barcode,
     )
     return dcg_info
 

diff --git a/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py b/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py
@@ -6,9 +6,6 @@
 
 from dodal.devices.synchrotron import SynchrotronMode
 
-from hyperion.external_interaction.callbacks.common.ispyb_mapping import (
-    populate_data_collection_group,
-)
 from hyperion.external_interaction.callbacks.plan_reactive_callback import (
     PlanReactiveCallback,
 )
@@ -17,7 +14,6 @@
 )
 from hyperion.external_interaction.ispyb.data_model import (
     DataCollectionGridInfo,
-    DataCollectionGroupInfo,
     DataCollectionInfo,
     ScanDataInfo,
 )
@@ -55,7 +51,6 @@ def __init__(
         ISPYB_LOGGER.debug("Initialising ISPyB callback")
         super().__init__(log=ISPYB_LOGGER, emit=emit)
         self._oav_snapshot_event_idx: int = 0
-        self._sample_barcode: Optional[str] = None
         self.params: GridscanInternalParameters | RotationInternalParameters | None = (
             None
         )
@@ -77,7 +72,6 @@ def __init__(
 
     def activity_gated_start(self, doc: RunStart):
         self._oav_snapshot_event_idx = 0
-        self._sample_barcode = None
         return self._tag_doc(doc)
 
     def activity_gated_descriptor(self, doc: EventDescriptor):
@@ -98,27 +92,20 @@ def activity_gated_event(self, doc: Event) -> Event:
                 "has no corresponding descriptor record"
             )
             return doc
-        data_collection_group_info = None
         match event_descriptor.get("name"):
             case CONST.DESCRIPTORS.ISPYB_HARDWARE_READ:
-                data_collection_group_info, scan_data_infos = (
-                    self._handle_ispyb_hardware_read(doc)
-                )
+                scan_data_infos = self._handle_ispyb_hardware_read(doc)
             case CONST.DESCRIPTORS.OAV_SNAPSHOT_TRIGGERED:
                 scan_data_infos = self._handle_oav_snapshot_triggered(doc)
             case CONST.DESCRIPTORS.ISPYB_TRANSMISSION_FLUX_READ:
                 scan_data_infos = self._handle_ispyb_transmission_flux_read(doc)
             case _:
                 return self._tag_doc(doc)
-        self.ispyb_ids = self.ispyb.update_deposition(
-            self.ispyb_ids, data_collection_group_info, scan_data_infos
-        )
+        self.ispyb_ids = self.ispyb.update_deposition(self.ispyb_ids, scan_data_infos)
         ISPYB_LOGGER.info(f"Recieved ISPYB IDs: {self.ispyb_ids}")
         return self._tag_doc(doc)
 
-    def _handle_ispyb_hardware_read(
-        self, doc
-    ) -> tuple[DataCollectionGroupInfo, Sequence[ScanDataInfo]]:
+    def _handle_ispyb_hardware_read(self, doc) -> Sequence[ScanDataInfo]:
         assert self.params, "Event handled before activity_gated_start received params"
         ISPYB_LOGGER.info("ISPyB handler received event from read hardware")
         assert isinstance(
@@ -131,20 +118,11 @@ def _handle_ispyb_hardware_read(
             slitgap_horizontal=doc["data"]["s4_slit_gaps_xgap"],
             slitgap_vertical=doc["data"]["s4_slit_gaps_ygap"],
         )
-        self._sample_barcode = doc["data"]["robot-barcode"]
         scan_data_infos = self.populate_info_for_update(
             hwscan_data_collection_info, self.params
         )
-        ISPYB_LOGGER.info(
-            "Updating ispyb data collection and group after hardware read."
-        )
-        data_collection_group_info = populate_data_collection_group(
-            self.ispyb.experiment_type,
-            self.params.hyperion_params.detector_params,
-            self.params.hyperion_params.ispyb_params,
-            self._sample_barcode,
-        )
-        return data_collection_group_info, scan_data_infos
+        ISPYB_LOGGER.info("Updating ispyb data collection after hardware read.")
+        return scan_data_infos
 
     def _handle_oav_snapshot_triggered(self, doc) -> Sequence[ScanDataInfo]:
         assert self.ispyb_ids.data_collection_ids, "No current data collection"
@@ -186,9 +164,7 @@ def _handle_oav_snapshot_triggered(self, doc) -> Sequence[ScanDataInfo]:
             data_collection_id=data_collection_id,
             data_collection_grid_info=data_collection_grid_info,
         )
-        ISPYB_LOGGER.info(
-            "Updating ispyb data collection and group after oav snapshot."
-        )
+        ISPYB_LOGGER.info("Updating ispyb data collection after oav snapshot.")
         self._oav_snapshot_event_idx += 1
         return [scan_data_info]
 
@@ -209,23 +185,6 @@ def _handle_ispyb_transmission_flux_read(self, doc) -> Sequence[ScanDataInfo]:
         ISPYB_LOGGER.info("Updating ispyb data collection after flux read.")
         return scan_data_infos
 
-    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 populate_info_for_update(
         self,

diff --git a/src/hyperion/external_interaction/ispyb/ispyb_store.py b/src/hyperion/external_interaction/ispyb/ispyb_store.py
@@ -66,7 +66,6 @@ def begin_deposition(
     def update_deposition(
         self,
         ispyb_ids,
-        data_collection_group_info: Optional[DataCollectionGroupInfo],
         scan_data_infos: Sequence[ScanDataInfo],
     ) -> IspybIds:
         assert (
@@ -75,9 +74,7 @@ def update_deposition(
         assert (
             ispyb_ids.data_collection_ids
         ), "Attempted to store scan data without a collection"
-        return self._begin_or_update_deposition(
-            ispyb_ids, data_collection_group_info, scan_data_infos
-        )
+        return self._begin_or_update_deposition(ispyb_ids, None, scan_data_infos)
 
     def _begin_or_update_deposition(
         self,

diff --git a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py
@@ -494,9 +494,7 @@ def test_ispyb_deposition_comment_correct_for_3D_on_failure(
         ExperimentType.GRIDSCAN_3D,
         ispyb_ids,
     )
-    ispyb_ids = dummy_ispyb_3d.update_deposition(
-        ispyb_ids, dummy_data_collection_group_info, scan_data_infos
-    )
+    ispyb_ids = dummy_ispyb_3d.update_deposition(ispyb_ids, scan_data_infos)
     dcid1 = ispyb_ids.data_collection_ids[0]  # type: ignore
     dcid2 = ispyb_ids.data_collection_ids[1]  # type: ignore
     dummy_ispyb_3d.end_deposition(ispyb_ids, "fail", "could not connect to devices")
@@ -539,9 +537,7 @@ def test_can_store_2D_ispyb_data_correctly_when_in_error(
         dummy_params, dummy_scan_data_info_for_begin, experiment_type, ispyb_ids
     )
 
-    ispyb_ids = ispyb.update_deposition(
-        ispyb_ids, dummy_data_collection_group_info, scan_data_infos
-    )
+    ispyb_ids = ispyb.update_deposition(ispyb_ids, scan_data_infos)
     assert len(ispyb_ids.data_collection_ids) == exp_num_of_grids  # type: ignore
     assert len(ispyb_ids.grid_ids) == exp_num_of_grids  # type: ignore
     assert isinstance(ispyb_ids.data_collection_group_id, int)
@@ -893,5 +889,3 @@ def test_ispyb_deposition_in_rotation_plan(
         fetch_datacollection_attribute(dcid, "slitGapHorizontal") == test_slit_gap_horiz
     )
     assert fetch_datacollection_attribute(dcid, "slitGapVertical") == test_slit_gap_vert
-    # TODO Can't test barcode as need BLSample which needs Dewar, Shipping, Container entries for the
-    # upsert stored proc to use it.
diff --git a/tests/test_data/parameter_json_files/good_test_robot_load_params.json b/tests/test_data/parameter_json_files/good_test_robot_load_params.json
@@ -24,8 +24,7 @@
             "focal_spot_size_x": 0.0,
             "focal_spot_size_y": 0.0,
             "comment": "Descriptive comment.",
-            "sample_id": 12345,
-            "sample_barcode": null
+            "sample_id": 12345
         }
     },
     "experiment_params": {

diff --git a/tests/unit_tests/experiment_plans/conftest.py b/tests/unit_tests/experiment_plans/conftest.py
@@ -47,7 +47,6 @@ def make_event_doc(data, descriptor="abc123") -> Event:
     "synchrotron-synchrotron_mode": SynchrotronMode.USER,
     "s4_slit_gaps_xgap": 0,
     "s4_slit_gaps_ygap": 0,
-    "robot-barcode": "BARCODE",
 }
 
 BASIC_POST_SETUP_DOC = {

diff --git a/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py b/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py
@@ -214,7 +214,6 @@ def test_read_hardware_for_ispyb_updates_from_ophyd_devices(
         fake_fgs_composite.aperture_scatterguard.set(
             fake_fgs_composite.aperture_scatterguard.aperture_positions.LARGE  # type: ignore
         )
-        set_sim_value(fake_fgs_composite.robot.barcode.bare_signal, ["BARCODE"])
 
         test_ispyb_callback = PlanReactiveCallback(ISPYB_LOGGER)
         test_ispyb_callback.active = True
@@ -253,7 +252,6 @@ def test_read_hardware_for_ispyb_updates_from_ophyd_devices(
                     "synchrotron-synchrotron_mode": synchrotron_test_value.value,
                     "s4_slit_gaps_xgap": xgap_test_value,
                     "s4_slit_gaps_ygap": ygap_test_value,
-                    "robot-barcode": "BARCODE",
                 },
             )
             assert_event(

diff --git a/tests/unit_tests/experiment_plans/test_panda_flyscan_xray_centre_plan.py b/tests/unit_tests/experiment_plans/test_panda_flyscan_xray_centre_plan.py
@@ -161,8 +161,6 @@ def test_read_hardware_for_ispyb_updates_from_ophyd_devices(
         flux_test_value = 10.0
         fake_fgs_composite.flux.flux_reading.sim_put(flux_test_value)  # type: ignore
 
-        set_sim_value(fake_fgs_composite.robot.barcode.bare_signal, ["BARCODE"])
-
         test_ispyb_callback = PlanReactiveCallback(ISPYB_LOGGER)
         test_ispyb_callback.active = True
         with patch.multiple(
@@ -199,7 +197,6 @@ def test_read_hardware_for_ispyb_updates_from_ophyd_devices(
                     "synchrotron-synchrotron_mode": synchrotron_test_value.value,
                     "s4_slit_gaps_xgap": xgap_test_value,
                     "s4_slit_gaps_ygap": ygap_test_value,
-                    "robot-barcode": "BARCODE",
                 },
             )
             assert_event(

diff --git a/tests/unit_tests/external_interaction/callbacks/conftest.py b/tests/unit_tests/external_interaction/callbacks/conftest.py
@@ -196,7 +196,6 @@ class TestData:
             "s4_slit_gaps_ygap": 0.2345,
             "synchrotron-synchrotron_mode": SynchrotronMode.USER,
             "undulator_current_gap": 1.234,
-            "robot-barcode": "BARCODE",
         },
         "timestamps": {"det1": 1666604299.8220396, "det2": 1666604299.8235943},
         "seq_num": 1,

diff --git a/tests/unit_tests/external_interaction/callbacks/rotation/test_ispyb_callback.py b/tests/unit_tests/external_interaction/callbacks/rotation/test_ispyb_callback.py
@@ -99,17 +99,7 @@ def test_hardware_read_events(
         TestData.test_descriptor_document_pre_data_collection
     )
     callback.activity_gated_event(TestData.test_event_document_pre_data_collection)
-    assert_upsert_call_with(
-        mx.upsert_data_collection_group.mock_calls[0],
-        mx.get_data_collection_group_params(),
-        {
-            "id": TEST_DATA_COLLECTION_GROUP_ID,
-            "parentid": TEST_SESSION_ID,
-            "experimenttype": "SAD",
-            "sampleid": TEST_SAMPLE_ID,
-            "sample_barcode": "BARCODE",
-        },
-    )
+    mx.upsert_data_collection_group.assert_not_called()
     assert_upsert_call_with(
         mx.upsert_data_collection.mock_calls[0],
         mx.get_data_collection_params(),

diff --git a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_callback.py b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_ispyb_callback.py
@@ -134,17 +134,7 @@ def test_hardware_read_event_3d(self, mock_ispyb_conn):
             TestData.test_descriptor_document_pre_data_collection
         )
         callback.activity_gated_event(TestData.test_event_document_pre_data_collection)
-        assert_upsert_call_with(
-            mx_acq.upsert_data_collection_group.mock_calls[0],  # pyright: ignore
-            mx_acq.get_data_collection_group_params(),
-            {
-                "id": TEST_DATA_COLLECTION_GROUP_ID,
-                "parentid": TEST_SESSION_ID,
-                "experimenttype": "Mesh3D",
-                "sampleid": TEST_SAMPLE_ID,
-                "sample_barcode": "BARCODE",  # deferred
-            },
-        )
+        mx_acq.upsert_data_collection_group.assert_not_called()
         assert_upsert_call_with(
             mx_acq.upsert_data_collection.mock_calls[0],
             mx_acq.get_data_collection_params(),