Remove unused ispyb extras (#493)

* Remove unused ispyb extras * Update parameter version * Fix some typing
DiamondLightSource · Sep 16, 2024 · c1c7b55 · c1c7b55
1 parent 470361a
commit c1c7b55
Show file tree

Hide file tree

Showing 9 changed files with 27 additions and 103 deletions.
diff --git a/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py b/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py
@@ -353,7 +353,7 @@ def rotation_scan(
         md={
             "subplan_name": CONST.PLAN.ROTATION_OUTER,
             CONST.TRIGGER.ZOCALO: CONST.PLAN.ROTATION_MAIN,
-            "hyperion_parameters": parameters.model_dump_json,
+            "hyperion_parameters": parameters.model_dump_json(),
             "activate_callbacks": [
                 "RotationISPyBCallback",
                 "RotationNexusFileCallback",

diff --git a/src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py b/src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py
@@ -1,8 +1,6 @@
 from __future__ import annotations
 
 from mx_bluesky.hyperion.external_interaction.ispyb.data_model import DataCollectionInfo
-from mx_bluesky.hyperion.log import ISPYB_LOGGER
-from mx_bluesky.hyperion.parameters.components import TemporaryIspybExtras
 from mx_bluesky.hyperion.parameters.rotation import RotationScan
 
 
@@ -16,24 +14,4 @@ def populate_data_collection_info_for_rotation(params: RotationScan):
         axis_end=(params.omega_start_deg + params.scan_width_deg),
         kappa_start=params.kappa_start_deg,
     )
-    (
-        info.xtal_snapshot1,
-        info.xtal_snapshot2,
-        info.xtal_snapshot3,
-        info.xtal_snapshot4,
-    ) = get_xtal_snapshots(params.ispyb_extras)
     return info
-
-
-def get_xtal_snapshots(ispyb_extras: TemporaryIspybExtras | None):
-    if ispyb_extras and ispyb_extras.xtal_snapshots_omega_start:
-        xtal_snapshots = ispyb_extras.xtal_snapshots_omega_start[:4]
-        ISPYB_LOGGER.info(
-            f"Using rotation scan snapshots {xtal_snapshots} for ISPyB deposition"
-        )
-    else:
-        ISPYB_LOGGER.info(
-            "No xtal snapshot paths sent to ISPyB from GDA, will take own snapshots"
-        )
-        xtal_snapshots = []
-    return xtal_snapshots + [None] * (4 - len(xtal_snapshots))
diff --git a/src/mx_bluesky/hyperion/parameters/components.py b/src/mx_bluesky/hyperion/parameters/components.py
@@ -38,7 +38,7 @@ def _parse(cls, version):
         return cls.parse(version)
 
 
-PARAMETER_VERSION = ParameterVersion.parse("5.0.0")
+PARAMETER_VERSION = ParameterVersion.parse("5.1.0")
 
 
 class RotationAxis(StrEnum):
@@ -251,9 +251,3 @@ class OptionalGonioAngleStarts(BaseModel):
     phi_start_deg: float | None = None
     chi_start_deg: float | None = None
     kappa_start_deg: float | None = None
-
-
-class TemporaryIspybExtras(BaseModel):
-    model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid")
-
-    xtal_snapshots_omega_start: list[str] | None = None
diff --git a/src/mx_bluesky/hyperion/parameters/rotation.py b/src/mx_bluesky/hyperion/parameters/rotation.py
@@ -24,7 +24,6 @@
     OptionalXyzStarts,
     RotationAxis,
     SplitScan,
-    TemporaryIspybExtras,
     WithScan,
 )
 from mx_bluesky.hyperion.parameters.constants import (
@@ -39,7 +38,6 @@ class RotationScanPerSweep(OptionalGonioAngleStarts, OptionalXyzStarts):
     scan_width_deg: float = Field(default=360, gt=0)
     rotation_direction: RotationDirection = Field(default=RotationDirection.NEGATIVE)
     nexus_vds_start_img: int = Field(default=0, ge=0)
-    ispyb_extras: TemporaryIspybExtras | None = None
 
 
 class RotationExperiment(DiffractionExperimentWithSample):

diff --git a/tests/conftest.py b/tests/conftest.py
@@ -348,6 +348,7 @@ def oav(test_config_files):
     )
     oav = i03.oav(fake_with_ophyd_sim=True, params=parameters)
     oav.snapshot.trigger = MagicMock(return_value=NullStatus())
+    oav.grid_snapshot.trigger = MagicMock(return_value=NullStatus())
     return oav
 
 

diff --git a/tests/system_tests/hyperion/external_interaction/test_ispyb_dev_connection.py b/tests/system_tests/hyperion/external_interaction/test_ispyb_dev_connection.py
@@ -11,6 +11,7 @@
 import numpy
 import pytest
 from bluesky.run_engine import RunEngine
+from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.oav.oav_parameters import OAVParameters
 from dodal.devices.synchrotron import SynchrotronMode
 from ophyd.sim import NullStatus
@@ -254,10 +255,12 @@ def grid_detect_then_xray_centre_composite(
     zebra,
     eiger,
     robot,
-    oav,
+    oav: OAV,
     dcm,
     flux,
     ophyd_pin_tip_detection,
+    sample_shutter,
+    done_status,
 ):
     composite = GridDetectThenXRayCentreComposite(
         zebra_fast_grid_scan=fast_grid_scan,
@@ -280,12 +283,13 @@ def grid_detect_then_xray_centre_composite(
         oav=oav,
         dcm=dcm,
         flux=flux,
+        sample_shutter=sample_shutter,
     )
     oav.zoom_controller.zrst.set("1.0x")
-    oav.cam.array_size.array_size_x.sim_put(1024)
-    oav.cam.array_size.array_size_y.sim_put(768)
-    oav.grid_snapshot.x_size.sim_put(1024)
-    oav.grid_snapshot.y_size.sim_put(768)
+    oav.cam.array_size.array_size_x.sim_put(1024)  # type: ignore
+    oav.cam.array_size.array_size_y.sim_put(768)  # type: ignore
+    oav.grid_snapshot.x_size.sim_put(1024)  # type: ignore
+    oav.grid_snapshot.y_size.sim_put(768)  # type: ignore
     oav.grid_snapshot.top_left_x.set(50)
     oav.grid_snapshot.top_left_y.set(100)
     oav.grid_snapshot.box_width.set(0.1 * 1000 / 1.25)  # size in pixels
@@ -605,6 +609,9 @@ def test_can_store_2D_ispyb_data_correctly_when_in_error(
 
 
 @pytest.mark.s03
+@pytest.mark.skip(
+    "Broken, fix in https://github.com/DiamondLightSource/mx-bluesky/issues/183"
+)
 def test_ispyb_deposition_in_gridscan(
     RE: RunEngine,
     grid_detect_then_xray_centre_composite: GridDetectThenXRayCentreComposite,
@@ -664,7 +671,7 @@ def test_ispyb_deposition_in_gridscan(
         ispyb_ids.data_collection_ids[0],
         "Hyperion: Xray centring - Diffraction grid scan of 20 by 12 "
         "images in 20.0 um by 20.0 um steps. Top left (px): [100,161], "
-        "bottom right (px): [239,244]. Aperture: Small. ",
+        "bottom right (px): [239,244]. ApertureValue.SMALL. ",
     )
     compare_actual_and_expected(
         ispyb_ids.data_collection_ids[0],
@@ -718,7 +725,7 @@ def test_ispyb_deposition_in_gridscan(
         ispyb_ids.data_collection_ids[1],
         "Hyperion: Xray centring - Diffraction grid scan of 20 by 11 "
         "images in 20.0 um by 20.0 um steps. Top left (px): [100,165], "
-        "bottom right (px): [239,241]. Aperture: Small. ",
+        "bottom right (px): [239,241]. ApertureValue.SMALL. ",
     )
     position_id = fetch_datacollection_attribute(
         ispyb_ids.data_collection_ids[1], DATA_COLLECTION_COLUMN_MAP["positionid"]
@@ -767,7 +774,7 @@ def test_ispyb_deposition_in_rotation_plan(
     assert dcid is not None
     assert (
         fetch_comment(dcid)
-        == "Sample position (µm): (1000, 2000, 3000) test  Aperture: Small. "
+        == "Sample position (µm): (1, 2, 3) test  Aperture: ApertureValue.SMALL. "
     )
 
     expected_values = EXPECTED_DATACOLLECTION_FOR_ROTATION | {
@@ -782,48 +789,12 @@ def test_ispyb_deposition_in_rotation_plan(
     position_id = fetch_datacollection_attribute(
         dcid, DATA_COLLECTION_COLUMN_MAP["positionid"]
     )
-    expected_values = {"posX": 1.0, "posY": 2.0, "posZ": 3.0}
+    expected_values = {"posX": 0.001, "posY": 0.002, "posZ": 0.003}
     compare_actual_and_expected(
         position_id, expected_values, fetch_datacollection_position_attribute
     )
 
 
-@pytest.mark.s03
-def test_ispyb_deposition_in_rotation_plan_snapshots_in_parameters(
-    composite_for_rotation_scan: RotationScanComposite,
-    params_for_rotation_scan: RotationScan,
-    oav_parameters_for_rotation: OAVParameters,
-    RE: RunEngine,
-    fetch_datacollection_attribute: Callable[..., Any],
-):
-    os.environ["ISPYB_CONFIG_PATH"] = CONST.SIM.DEV_ISPYB_DATABASE_CFG
-    ispyb_cb = RotationISPyBCallback()
-    RE.subscribe(ispyb_cb)
-    params_for_rotation_scan.snapshot_omegas_deg = None
-    params_for_rotation_scan.ispyb_extras.xtal_snapshots_omega_start = [  # type: ignore
-        "/tmp/test_snapshot1.png",
-        "/tmp/test_snapshot2.png",
-        "/tmp/test_snapshot3.png",
-    ]
-    RE(
-        rotation_scan(
-            composite_for_rotation_scan,
-            params_for_rotation_scan,
-            oav_parameters_for_rotation,
-        )
-    )
-
-    dcid = ispyb_cb.ispyb_ids.data_collection_ids[0]
-    assert dcid is not None
-    expected_values = EXPECTED_DATACOLLECTION_FOR_ROTATION | {
-        "xtalSnapshotFullPath1": "/tmp/test_snapshot1.png",
-        "xtalSnapshotFullPath2": "/tmp/test_snapshot2.png",
-        "xtalSnapshotFullPath3": "/tmp/test_snapshot3.png",
-    }
-
-    compare_actual_and_expected(dcid, expected_values, fetch_datacollection_attribute)
-
-
 def generate_scan_data_infos(
     dummy_params,
     dummy_scan_data_info_for_begin: ScanDataInfo,

diff --git a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json
@@ -25,12 +25,10 @@
     "y_start_um": 2.0,
     "z_start_um": 3.0,
     "selected_aperture": "SMALL_APERTURE",
-    "snapshot_omegas_deg": [0, 90, 180, 270],
-    "ispyb_extras": {
-        "xtal_snapshots_omega_start": [
-            "test_1_y",
-            "test_2_y",
-            "test_3_y"
-        ]
-    }
+    "snapshot_omegas_deg": [
+        0,
+        90,
+        180,
+        270
+    ]
 }
diff --git a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters_nomove.json b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters_nomove.json
@@ -19,12 +19,5 @@
     "visit": "cm31105-4",
     "zocalo_environment": "dev_artemis",
     "transmission_frac": 0.1,
-    "selected_aperture": "LARGE_APERTURE",
-    "ispyb_extras": {
-        "xtal_snapshots_omega_start": [
-            "test_1_y",
-            "test_2_y",
-            "test_3_y"
-        ]
-    }
+    "selected_aperture": "LARGE_APERTURE"
 }
diff --git a/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py b/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py
@@ -51,10 +51,6 @@
 def rotation_start_outer_doc_without_snapshots(
     test_rotation_start_outer_document, dummy_rotation_params
 ):
-    dummy_rotation_params.ispyb_extras.xtal_snapshots_omega_start = None
-    test_rotation_start_outer_document["hyperion_parameters"] = (
-        dummy_rotation_params.model_dump_json()
-    )
     return test_rotation_start_outer_document
 
 
@@ -108,12 +104,7 @@ def test_activity_gated_start_with_snapshot_parameters(
     assert_upsert_call_with(
         mx.upsert_data_collection.mock_calls[0],
         mx.get_data_collection_params(),
-        EXPECTED_DATA_COLLECTION
-        | {
-            "xtal_snapshot1": "test_1_y",
-            "xtal_snapshot2": "test_2_y",
-            "xtal_snapshot3": "test_3_y",
-        },
+        EXPECTED_DATA_COLLECTION,
     )