From 60aa755122e73a7ebadfa2a34d67f1497c4e53fd Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 7 Mar 2024 13:05:26 +0000 Subject: [PATCH 01/23] #1231 move util scripts into subdir and update readme --- README.md | 2 +- beam_off_trickery.sh => utility_scripts/beam_off_trickery.sh | 0 {deploy => utility_scripts/deploy}/deploy_hyperion.py | 0 .../dev_jaeger_container.sh | 0 dls_dev_env.sh => utility_scripts/dls_dev_env.sh | 0 setup_graylog.sh => utility_scripts/setup_graylog.sh | 0 6 files changed, 1 insertion(+), 1 deletion(-) rename beam_off_trickery.sh => utility_scripts/beam_off_trickery.sh (100%) rename {deploy => utility_scripts/deploy}/deploy_hyperion.py (100%) rename dev_jaeger_container.sh => utility_scripts/dev_jaeger_container.sh (100%) rename dls_dev_env.sh => utility_scripts/dls_dev_env.sh (100%) rename setup_graylog.sh => utility_scripts/setup_graylog.sh (100%) diff --git a/README.md b/README.md index 24b978475..22dd51488 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Left to do is: Development Installation ================= -Run `dls_dev_env.sh` (This assumes you're on a DLS machine. If you are not, you should be able to just run a subset of this script) +Run `./utility_scripts/dls_dev_env.sh` (This assumes you're on a DLS machine. If you are not, you should be able to just run a subset of this script) Note that because Hyperion makes heavy use of [Dodal](https://github.com/DiamondLightSource/dodal) this will also pull a local editable version of dodal to the parent folder of this repo. diff --git a/beam_off_trickery.sh b/utility_scripts/beam_off_trickery.sh similarity index 100% rename from beam_off_trickery.sh rename to utility_scripts/beam_off_trickery.sh diff --git a/deploy/deploy_hyperion.py b/utility_scripts/deploy/deploy_hyperion.py similarity index 100% rename from deploy/deploy_hyperion.py rename to utility_scripts/deploy/deploy_hyperion.py diff --git a/dev_jaeger_container.sh b/utility_scripts/dev_jaeger_container.sh similarity index 100% rename from dev_jaeger_container.sh rename to utility_scripts/dev_jaeger_container.sh diff --git a/dls_dev_env.sh b/utility_scripts/dls_dev_env.sh similarity index 100% rename from dls_dev_env.sh rename to utility_scripts/dls_dev_env.sh diff --git a/setup_graylog.sh b/utility_scripts/setup_graylog.sh similarity index 100% rename from setup_graylog.sh rename to utility_scripts/setup_graylog.sh From 318222da417ad11a5492bbb9a25f0027daf0cc97 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 7 Mar 2024 13:14:32 +0000 Subject: [PATCH 02/23] #1231 update dev env script for safety --- utility_scripts/dls_dev_env.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/utility_scripts/dls_dev_env.sh b/utility_scripts/dls_dev_env.sh index 3880ffa3b..ec882ced1 100755 --- a/utility_scripts/dls_dev_env.sh +++ b/utility_scripts/dls_dev_env.sh @@ -1,5 +1,12 @@ #!/bin/bash +# Check we're in the right place +dir_name=${PWD##*/} +if [ "$dir_name" != "hyperion" ]; then + echo "This script should be run from the 'hyperion' directory" + exit 1 +fi + # controls_dev sets pip up to look at a local pypi server, which is incomplete module unload controls_dev From ff7c0da3468da38af46c177a024e30e70bab27fb Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 7 Mar 2024 13:31:02 +0000 Subject: [PATCH 03/23] #1231 update deploy script --- utility_scripts/deploy/deploy_hyperion.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/utility_scripts/deploy/deploy_hyperion.py b/utility_scripts/deploy/deploy_hyperion.py index 1e75f70df..4d97df91a 100644 --- a/utility_scripts/deploy/deploy_hyperion.py +++ b/utility_scripts/deploy/deploy_hyperion.py @@ -74,7 +74,7 @@ def get_hyperion_release_dir_from_args(repo: repo) -> str: if __name__ == "__main__": hyperion_repo = repo( name="hyperion", - repo_args=os.path.join(os.path.dirname(__file__), "../.git"), + repo_args=os.path.join(os.path.dirname(__file__), "../../.git"), ) # Gives path to /bluesky @@ -88,7 +88,7 @@ def get_hyperion_release_dir_from_args(repo: repo) -> str: dodal_repo = repo( name="dodal", - repo_args=os.path.join(os.path.dirname(__file__), "../../dodal/.git"), + repo_args=os.path.join(os.path.dirname(__file__), "../../../dodal/.git"), ) dodal_repo.set_deploy_location(release_area_version) @@ -114,7 +114,10 @@ def get_hyperion_release_dir_from_args(repo: repo) -> str: if hyperion_repo.name == "hyperion": with Popen( - "./dls_dev_env.sh", stdout=PIPE, bufsize=1, universal_newlines=True + "./utility_scripts/dls_dev_env.sh", + stdout=PIPE, + bufsize=1, + universal_newlines=True, ) as p: if p.stdout is not None: for line in p.stdout: From 4f8dad8fcff4151293fe0814551930306bda534d Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 7 Mar 2024 13:42:23 +0000 Subject: [PATCH 04/23] #1226 use new symlink locations for deployment --- utility_scripts/deploy/deploy_hyperion.py | 33 ++++++++++++----------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/utility_scripts/deploy/deploy_hyperion.py b/utility_scripts/deploy/deploy_hyperion.py index 4d97df91a..da13ae8e6 100644 --- a/utility_scripts/deploy/deploy_hyperion.py +++ b/utility_scripts/deploy/deploy_hyperion.py @@ -1,6 +1,5 @@ import argparse import os -from subprocess import PIPE, CalledProcessError, Popen from git import Repo from packaging.version import Version @@ -112,32 +111,36 @@ def get_hyperion_release_dir_from_args(repo: repo) -> str: os.chdir(hyperion_repo.deploy_location) print(f"Setting up environment in {hyperion_repo.deploy_location}") - if hyperion_repo.name == "hyperion": - with Popen( - "./utility_scripts/dls_dev_env.sh", - stdout=PIPE, - bufsize=1, - universal_newlines=True, - ) as p: - if p.stdout is not None: - for line in p.stdout: - print(line, end="") + # if hyperion_repo.name == "hyperion": + # with Popen( + # "./utility_scripts/dls_dev_env.sh", + # stdout=PIPE, + # bufsize=1, + # universal_newlines=True, + # ) as p: + # if p.stdout is not None: + # for line in p.stdout: + # print(line, end="") - if p.returncode != 0: - raise CalledProcessError(p.returncode, p.args) + # if p.returncode != 0: + # raise CalledProcessError(p.returncode, p.args) move_symlink = input( """Move symlink (y/n)? WARNING: this will affect the running version! Only do so if you have informed the beamline scientist and you're sure Hyperion is not running. """ ) - # Creates symlink: software/bluesky/hyperion_version -> software/bluesky/hyperion + # Creates symlinks: software/bluesky/hyperion_latest -> software/bluesky/hyperion_{version}/hyperion + # software/bluesky/hyperion -> software/bluesky/hyperion if move_symlink == "y": + latest_location = os.path.join(release_area, "hyperion_latest") live_location = os.path.join(release_area, "hyperion") new_tmp_location = os.path.join(release_area, "tmp_art") os.symlink(hyperion_repo.deploy_location, new_tmp_location) + os.rename(new_tmp_location, latest_location) + os.symlink(latest_location, new_tmp_location) os.rename(new_tmp_location, live_location) - print(f"New version moved to {live_location}") + print(f"New version moved to {latest_location}") print("To start this version run hyperion_restart from the beamline's GDA") else: print("Quiting without latest version being updated") From fd9c6c86cda733edb1eb30e85d031f923c3deaec Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 7 Mar 2024 13:44:42 +0000 Subject: [PATCH 05/23] #1226 uncomment env section in deploy script --- utility_scripts/deploy/deploy_hyperion.py | 27 ++++++++++++----------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/utility_scripts/deploy/deploy_hyperion.py b/utility_scripts/deploy/deploy_hyperion.py index da13ae8e6..98561f340 100644 --- a/utility_scripts/deploy/deploy_hyperion.py +++ b/utility_scripts/deploy/deploy_hyperion.py @@ -1,5 +1,6 @@ import argparse import os +from subprocess import PIPE, CalledProcessError, Popen from git import Repo from packaging.version import Version @@ -111,19 +112,19 @@ def get_hyperion_release_dir_from_args(repo: repo) -> str: os.chdir(hyperion_repo.deploy_location) print(f"Setting up environment in {hyperion_repo.deploy_location}") - # if hyperion_repo.name == "hyperion": - # with Popen( - # "./utility_scripts/dls_dev_env.sh", - # stdout=PIPE, - # bufsize=1, - # universal_newlines=True, - # ) as p: - # if p.stdout is not None: - # for line in p.stdout: - # print(line, end="") - - # if p.returncode != 0: - # raise CalledProcessError(p.returncode, p.args) + if hyperion_repo.name == "hyperion": + with Popen( + "./utility_scripts/dls_dev_env.sh", + stdout=PIPE, + bufsize=1, + universal_newlines=True, + ) as p: + if p.stdout is not None: + for line in p.stdout: + print(line, end="") + + if p.returncode != 0: + raise CalledProcessError(p.returncode, p.args) move_symlink = input( """Move symlink (y/n)? WARNING: this will affect the running version! From d7735e7536fe69bb04de9235a40609743e65b265 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 14 Mar 2024 11:17:36 +0000 Subject: [PATCH 06/23] #1199 remove callback collections and replace with simple lists --- src/hyperion/__main__.py | 25 ++++++++----- .../experiment_plans/experiment_registry.py | 26 ++++++-------- .../flyscan_xray_centre_plan.py | 3 -- .../abstract_plan_callback_collection.py | 25 ------------- .../callbacks/common/callback_util.py | 36 +++++++++++++++++++ .../callbacks/rotation/callback_collection.py | 31 ---------------- .../xray_centre/callback_collection.py | 32 ----------------- 7 files changed, 63 insertions(+), 115 deletions(-) delete mode 100644 src/hyperion/external_interaction/callbacks/abstract_plan_callback_collection.py create mode 100644 src/hyperion/external_interaction/callbacks/common/callback_util.py delete mode 100644 src/hyperion/external_interaction/callbacks/rotation/callback_collection.py delete mode 100644 src/hyperion/external_interaction/callbacks/xray_centre/callback_collection.py diff --git a/src/hyperion/__main__.py b/src/hyperion/__main__.py index 29db53333..140d6e381 100755 --- a/src/hyperion/__main__.py +++ b/src/hyperion/__main__.py @@ -13,13 +13,14 @@ from pydantic.dataclasses import dataclass from hyperion.exceptions import WarningException -from hyperion.experiment_plans.experiment_registry import PLAN_REGISTRY, PlanNotFound +from hyperion.experiment_plans.experiment_registry import ( + PLAN_REGISTRY, + CallbackFactories, + PlanNotFound, +) from hyperion.external_interaction.callbacks.__main__ import ( setup_logging as setup_callback_logging, ) -from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import ( - AbstractPlanCallbackCollection, -) from hyperion.external_interaction.callbacks.aperture_change_callback import ( ApertureChangeCallback, ) @@ -45,7 +46,7 @@ class Command: devices: Optional[Any] = None experiment: Optional[Callable[[Any, Any], MsgGenerator]] = None parameters: Optional[InternalParameters] = None - callbacks: Optional[type[AbstractPlanCallbackCollection]] = None + callbacks: Optional[CallbackFactories] = None @dataclass @@ -108,7 +109,7 @@ def start( experiment: Callable, parameters: InternalParameters, plan_name: str, - callback_type: Optional[type[AbstractPlanCallbackCollection]], + callbacks: Optional[CallbackFactories], ) -> StatusAndMessage: LOGGER.info(f"Started with parameters: {parameters}") @@ -122,7 +123,13 @@ def start( else: self.current_status = StatusAndMessage(Status.BUSY) self.command_queue.put( - Command(Actions.START, devices, experiment, parameters, callback_type) + Command( + action=Actions.START, + devices=devices, + experiment=experiment, + parameters=parameters, + callbacks=callbacks, + ) ) return StatusAndMessage(Status.SUCCESS) @@ -149,7 +156,7 @@ def shutdown(self): """Stops the run engine and the loop waiting for messages.""" print("Shutting down: Stopping the run engine gracefully") self.stop() - self.command_queue.put(Command(Actions.SHUTDOWN)) + self.command_queue.put(Command(action=Actions.SHUTDOWN)) def wait_on_queue(self): while True: @@ -163,7 +170,7 @@ def wait_on_queue(self): if ( not self.use_external_callbacks and command.callbacks - and (cbs := list(command.callbacks())) + and (cbs := [c() for c in command.callbacks]) ): LOGGER.info( f"Using callbacks for this plan: {not self.use_external_callbacks} - {cbs}" diff --git a/src/hyperion/experiment_plans/experiment_registry.py b/src/hyperion/experiment_plans/experiment_registry.py index f33be4d5c..ec7429f41 100644 --- a/src/hyperion/experiment_plans/experiment_registry.py +++ b/src/hyperion/experiment_plans/experiment_registry.py @@ -14,14 +14,10 @@ pin_centre_then_xray_centre_plan, wait_for_robot_load_then_centre_plan, ) -from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import ( - AbstractPlanCallbackCollection, -) -from hyperion.external_interaction.callbacks.rotation.callback_collection import ( - RotationCallbackCollection, -) -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, +from hyperion.external_interaction.callbacks.common.callback_util import ( + CallbackFactories, + gridscan_callbacks, + rotation_callbacks, ) from hyperion.parameters.plan_specific.grid_scan_with_edge_detect_params import ( GridScanWithEdgeDetectInternalParameters, @@ -66,7 +62,7 @@ class ExperimentRegistryEntry(TypedDict): | PandAGridscanInternalParameters ] experiment_param_type: type[AbstractExperimentParameterBase] - callback_collection_type: type[AbstractPlanCallbackCollection] + callback_factories: CallbackFactories EXPERIMENT_TYPES = Union[GridScanParams, RotationScanParams] @@ -75,37 +71,37 @@ class ExperimentRegistryEntry(TypedDict): "setup": panda_flyscan_xray_centre_plan.create_devices, "internal_param_type": PandAGridscanInternalParameters, "experiment_param_type": PandAGridScanParams, - "callback_collection_type": XrayCentreCallbackCollection, + "callback_factories": gridscan_callbacks, }, "flyscan_xray_centre": { "setup": flyscan_xray_centre_plan.create_devices, "internal_param_type": GridscanInternalParameters, "experiment_param_type": GridScanParams, - "callback_collection_type": XrayCentreCallbackCollection, + "callback_factories": gridscan_callbacks, }, "grid_detect_then_xray_centre": { "setup": grid_detect_then_xray_centre_plan.create_devices, "internal_param_type": GridScanWithEdgeDetectInternalParameters, "experiment_param_type": GridScanWithEdgeDetectParams, - "callback_collection_type": XrayCentreCallbackCollection, + "callback_factories": gridscan_callbacks, }, "rotation_scan": { "setup": rotation_scan_plan.create_devices, "internal_param_type": RotationInternalParameters, "experiment_param_type": RotationScanParams, - "callback_collection_type": RotationCallbackCollection, + "callback_factories": rotation_callbacks, }, "pin_tip_centre_then_xray_centre": { "setup": pin_centre_then_xray_centre_plan.create_devices, "internal_param_type": PinCentreThenXrayCentreInternalParameters, "experiment_param_type": PinCentreThenXrayCentreParams, - "callback_collection_type": XrayCentreCallbackCollection, + "callback_factories": gridscan_callbacks, }, "wait_for_robot_load_then_centre": { "setup": wait_for_robot_load_then_centre_plan.create_devices, "internal_param_type": WaitForRobotLoadThenCentreInternalParameters, "experiment_param_type": WaitForRobotLoadThenCentreParams, - "callback_collection_type": XrayCentreCallbackCollection, + "callback_factories": gridscan_callbacks, }, } EXPERIMENT_NAMES = list(PLAN_REGISTRY.keys()) diff --git a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py index a0e8cf938..56ae2b633 100755 --- a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py @@ -52,9 +52,6 @@ transmission_and_xbpm_feedback_for_collection_decorator, ) from hyperion.exceptions import WarningException -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, -) from hyperion.log import LOGGER from hyperion.parameters import external_parameters from hyperion.parameters.constants import CONST diff --git a/src/hyperion/external_interaction/callbacks/abstract_plan_callback_collection.py b/src/hyperion/external_interaction/callbacks/abstract_plan_callback_collection.py deleted file mode 100644 index 9475d793e..000000000 --- a/src/hyperion/external_interaction/callbacks/abstract_plan_callback_collection.py +++ /dev/null @@ -1,25 +0,0 @@ -from __future__ import annotations - -from abc import ABC -from dataclasses import fields -from typing import Any, Generator - -from bluesky.callbacks import CallbackBase - - -class AbstractPlanCallbackCollection(ABC): - """Base class for a collection of callbacks to attach to a plan. Subclasses should - also be dataclasses, or override __iter__. In general, you should use - '@dataclass(frozen=True, order=True)' for your subclass, in which case you can use - @subs_decorator(list(callback_collection)) to subscribe them to your plan in order. - """ - - def __iter__(self) -> Generator[CallbackBase, Any, None]: - for field in fields(self): # type: ignore # subclasses must be dataclass - yield getattr(self, field.name) - - -class NullPlanCallbackCollection(AbstractPlanCallbackCollection): - - def __iter__(self): - yield from () diff --git a/src/hyperion/external_interaction/callbacks/common/callback_util.py b/src/hyperion/external_interaction/callbacks/common/callback_util.py new file mode 100644 index 000000000..aec3eed99 --- /dev/null +++ b/src/hyperion/external_interaction/callbacks/common/callback_util.py @@ -0,0 +1,36 @@ +from typing import Callable, Sequence + +from bluesky.callbacks import CallbackBase + +from hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, +) +from hyperion.external_interaction.callbacks.rotation.nexus_callback import ( + RotationNexusFileCallback, +) +from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import ( + GridscanISPyBCallback, +) +from hyperion.external_interaction.callbacks.xray_centre.nexus_callback import ( + GridscanNexusFileCallback, +) +from hyperion.external_interaction.callbacks.zocalo_callback import ZocaloCallback + + +def gridscan_ispyb_with_zocalo(): + return GridscanISPyBCallback(emit=ZocaloCallback()) + + +def rotation_ispyb_with_zocalo(): + return RotationISPyBCallback(emit=ZocaloCallback()) + + +CallbackFactories = Sequence[Callable[[], CallbackBase]] +gridscan_callbacks: CallbackFactories = [ + GridscanNexusFileCallback, + gridscan_ispyb_with_zocalo, +] +rotation_callbacks: CallbackFactories = [ + RotationNexusFileCallback, + rotation_ispyb_with_zocalo, +] diff --git a/src/hyperion/external_interaction/callbacks/rotation/callback_collection.py b/src/hyperion/external_interaction/callbacks/rotation/callback_collection.py deleted file mode 100644 index a226e3071..000000000 --- a/src/hyperion/external_interaction/callbacks/rotation/callback_collection.py +++ /dev/null @@ -1,31 +0,0 @@ -from __future__ import annotations - -from dataclasses import dataclass, field - -from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import ( - AbstractPlanCallbackCollection, -) -from hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( - RotationISPyBCallback, -) -from hyperion.external_interaction.callbacks.rotation.nexus_callback import ( - RotationNexusFileCallback, -) -from hyperion.external_interaction.callbacks.zocalo_callback import ( - ZocaloCallback, -) - - -def new_ispyb_with_zocalo(): - return RotationISPyBCallback(emit=ZocaloCallback()) - - -@dataclass(frozen=True, order=True) -class RotationCallbackCollection(AbstractPlanCallbackCollection): - """Groups the callbacks for external interactions for a rotation scan. - Cast to a list to pass it to Bluesky.preprocessors.subs_decorator().""" - - nexus_handler: RotationNexusFileCallback = field( - default_factory=RotationNexusFileCallback - ) - ispyb_handler: RotationISPyBCallback = field(default_factory=new_ispyb_with_zocalo) diff --git a/src/hyperion/external_interaction/callbacks/xray_centre/callback_collection.py b/src/hyperion/external_interaction/callbacks/xray_centre/callback_collection.py deleted file mode 100644 index 7f1454c3f..000000000 --- a/src/hyperion/external_interaction/callbacks/xray_centre/callback_collection.py +++ /dev/null @@ -1,32 +0,0 @@ -from __future__ import annotations - -from dataclasses import dataclass, field - -from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import ( - AbstractPlanCallbackCollection, -) -from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import ( - GridscanISPyBCallback, -) -from hyperion.external_interaction.callbacks.xray_centre.nexus_callback import ( - GridscanNexusFileCallback, -) -from hyperion.external_interaction.callbacks.zocalo_callback import ( - ZocaloCallback, -) - - -def new_ispyb_with_zocalo(): - return GridscanISPyBCallback(emit=ZocaloCallback()) - - -@dataclass(frozen=True, order=True) -class XrayCentreCallbackCollection(AbstractPlanCallbackCollection): - """Groups the callbacks for external interactions in the fast grid scan, and - connects the Zocalo and ISPyB handlers. Cast to a list to pass it to - Bluesky.preprocessors.subs_decorator().""" - - nexus_handler: GridscanNexusFileCallback = field( - default_factory=GridscanNexusFileCallback - ) - ispyb_handler: GridscanISPyBCallback = field(default_factory=new_ispyb_with_zocalo) From 7b45b13743c4757ec59a65c5e98c78cdbf9f90bf Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 14 Mar 2024 11:29:27 +0000 Subject: [PATCH 07/23] #1199 simplify factories and use types tuple --- src/hyperion/__main__.py | 8 ++--- .../experiment_plans/experiment_registry.py | 22 +++++++------- .../flyscan_xray_centre_plan.py | 30 +------------------ .../callbacks/common/callback_util.py | 27 ++++++----------- .../test_zocalo_system.py | 6 ++-- 5 files changed, 28 insertions(+), 65 deletions(-) diff --git a/src/hyperion/__main__.py b/src/hyperion/__main__.py index 140d6e381..610f5aa20 100755 --- a/src/hyperion/__main__.py +++ b/src/hyperion/__main__.py @@ -6,6 +6,7 @@ from typing import Any, Callable, Optional, Tuple from blueapi.core import BlueskyContext, MsgGenerator +from bluesky.callbacks import CallbackBase from bluesky.callbacks.zmq import Publisher from bluesky.run_engine import RunEngine from flask import Flask, request @@ -15,7 +16,6 @@ from hyperion.exceptions import WarningException from hyperion.experiment_plans.experiment_registry import ( PLAN_REGISTRY, - CallbackFactories, PlanNotFound, ) from hyperion.external_interaction.callbacks.__main__ import ( @@ -46,7 +46,7 @@ class Command: devices: Optional[Any] = None experiment: Optional[Callable[[Any, Any], MsgGenerator]] = None parameters: Optional[InternalParameters] = None - callbacks: Optional[CallbackFactories] = None + callbacks: Optional[Callable[[], Tuple[CallbackBase, CallbackBase]]] = None @dataclass @@ -109,7 +109,7 @@ def start( experiment: Callable, parameters: InternalParameters, plan_name: str, - callbacks: Optional[CallbackFactories], + callbacks: Optional[Callable[[], Tuple[CallbackBase, CallbackBase]]], ) -> StatusAndMessage: LOGGER.info(f"Started with parameters: {parameters}") @@ -170,7 +170,7 @@ def wait_on_queue(self): if ( not self.use_external_callbacks and command.callbacks - and (cbs := [c() for c in command.callbacks]) + and (cbs := command.callbacks()) ): LOGGER.info( f"Using callbacks for this plan: {not self.use_external_callbacks} - {cbs}" diff --git a/src/hyperion/experiment_plans/experiment_registry.py b/src/hyperion/experiment_plans/experiment_registry.py index ec7429f41..981f38e49 100644 --- a/src/hyperion/experiment_plans/experiment_registry.py +++ b/src/hyperion/experiment_plans/experiment_registry.py @@ -1,7 +1,8 @@ from __future__ import annotations -from typing import Callable, TypedDict, Union +from typing import Callable, Tuple, TypedDict, Union +from bluesky.callbacks import CallbackBase from dodal.devices.fast_grid_scan import GridScanParams from dodal.devices.panda_fast_grid_scan import PandAGridScanParams from dodal.parameters.experiment_parameter_base import AbstractExperimentParameterBase @@ -15,9 +16,8 @@ wait_for_robot_load_then_centre_plan, ) from hyperion.external_interaction.callbacks.common.callback_util import ( - CallbackFactories, - gridscan_callbacks, - rotation_callbacks, + create_gridscan_callbacks, + create_rotation_callbacks, ) from hyperion.parameters.plan_specific.grid_scan_with_edge_detect_params import ( GridScanWithEdgeDetectInternalParameters, @@ -62,7 +62,7 @@ class ExperimentRegistryEntry(TypedDict): | PandAGridscanInternalParameters ] experiment_param_type: type[AbstractExperimentParameterBase] - callback_factories: CallbackFactories + callback_factories: Callable[[], Tuple[CallbackBase, CallbackBase]] EXPERIMENT_TYPES = Union[GridScanParams, RotationScanParams] @@ -71,37 +71,37 @@ class ExperimentRegistryEntry(TypedDict): "setup": panda_flyscan_xray_centre_plan.create_devices, "internal_param_type": PandAGridscanInternalParameters, "experiment_param_type": PandAGridScanParams, - "callback_factories": gridscan_callbacks, + "callback_factories": create_gridscan_callbacks, }, "flyscan_xray_centre": { "setup": flyscan_xray_centre_plan.create_devices, "internal_param_type": GridscanInternalParameters, "experiment_param_type": GridScanParams, - "callback_factories": gridscan_callbacks, + "callback_factories": create_gridscan_callbacks, }, "grid_detect_then_xray_centre": { "setup": grid_detect_then_xray_centre_plan.create_devices, "internal_param_type": GridScanWithEdgeDetectInternalParameters, "experiment_param_type": GridScanWithEdgeDetectParams, - "callback_factories": gridscan_callbacks, + "callback_factories": create_gridscan_callbacks, }, "rotation_scan": { "setup": rotation_scan_plan.create_devices, "internal_param_type": RotationInternalParameters, "experiment_param_type": RotationScanParams, - "callback_factories": rotation_callbacks, + "callback_factories": create_rotation_callbacks, }, "pin_tip_centre_then_xray_centre": { "setup": pin_centre_then_xray_centre_plan.create_devices, "internal_param_type": PinCentreThenXrayCentreInternalParameters, "experiment_param_type": PinCentreThenXrayCentreParams, - "callback_factories": gridscan_callbacks, + "callback_factories": create_gridscan_callbacks, }, "wait_for_robot_load_then_centre": { "setup": wait_for_robot_load_then_centre_plan.create_devices, "internal_param_type": WaitForRobotLoadThenCentreInternalParameters, "experiment_param_type": WaitForRobotLoadThenCentreParams, - "callback_factories": gridscan_callbacks, + "callback_factories": create_gridscan_callbacks, }, } EXPERIMENT_NAMES = list(PLAN_REGISTRY.keys()) diff --git a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py index 56ae2b633..cc85d7351 100755 --- a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py @@ -1,6 +1,5 @@ from __future__ import annotations -import argparse import dataclasses from typing import TYPE_CHECKING, Any, List @@ -8,8 +7,6 @@ import bluesky.preprocessors as bpp import numpy as np from blueapi.core import BlueskyContext, MsgGenerator -from bluesky.run_engine import RunEngine -from bluesky.utils import ProgressBarManager from dodal.devices.aperturescatterguard import ( ApertureScatterguard, SingleAperturePosition, @@ -53,13 +50,12 @@ ) from hyperion.exceptions import WarningException from hyperion.log import LOGGER -from hyperion.parameters import external_parameters from hyperion.parameters.constants import CONST from hyperion.tracing import TRACER from hyperion.utils.aperturescatterguard import ( load_default_aperture_scatterguard_positions_if_unset, ) -from hyperion.utils.context import device_composite_from_context, setup_context +from hyperion.utils.context import device_composite_from_context if TYPE_CHECKING: from hyperion.parameters.plan_specific.gridscan_internal_params import ( @@ -366,27 +362,3 @@ def run_gridscan_and_move_and_tidy(fgs_composite, params): yield from run_gridscan_and_move(fgs_composite, params) return run_gridscan_and_move_and_tidy(composite, parameters) - - -if __name__ == "__main__": - parser = argparse.ArgumentParser() - parser.add_argument( - "--beamline", - help="The beamline prefix this is being run on", - default=CONST.SIM.BEAMLINE, - ) - args = parser.parse_args() - - RE = RunEngine({}) - RE.waiting_hook = ProgressBarManager() # type: ignore - from hyperion.parameters.plan_specific.gridscan_internal_params import ( - GridscanInternalParameters, - ) - - parameters = GridscanInternalParameters(**external_parameters.conftest.from_file()) - subscriptions = XrayCentreCallbackCollection() - - context = setup_context(wait_for_connection=True) - composite = create_devices(context) - - RE(flyscan_xray_centre(composite, parameters)) diff --git a/src/hyperion/external_interaction/callbacks/common/callback_util.py b/src/hyperion/external_interaction/callbacks/common/callback_util.py index aec3eed99..4ae604eb1 100644 --- a/src/hyperion/external_interaction/callbacks/common/callback_util.py +++ b/src/hyperion/external_interaction/callbacks/common/callback_util.py @@ -1,6 +1,4 @@ -from typing import Callable, Sequence - -from bluesky.callbacks import CallbackBase +from typing import Tuple from hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( RotationISPyBCallback, @@ -17,20 +15,13 @@ from hyperion.external_interaction.callbacks.zocalo_callback import ZocaloCallback -def gridscan_ispyb_with_zocalo(): - return GridscanISPyBCallback(emit=ZocaloCallback()) - - -def rotation_ispyb_with_zocalo(): - return RotationISPyBCallback(emit=ZocaloCallback()) +def create_gridscan_callbacks() -> ( + Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] +): + return (GridscanNexusFileCallback(), GridscanISPyBCallback(emit=ZocaloCallback())) -CallbackFactories = Sequence[Callable[[], CallbackBase]] -gridscan_callbacks: CallbackFactories = [ - GridscanNexusFileCallback, - gridscan_ispyb_with_zocalo, -] -rotation_callbacks: CallbackFactories = [ - RotationNexusFileCallback, - rotation_ispyb_with_zocalo, -] +def create_rotation_callbacks() -> ( + Tuple[RotationNexusFileCallback, RotationISPyBCallback] +): + return (RotationNexusFileCallback(), RotationISPyBCallback(emit=ZocaloCallback())) diff --git a/tests/system_tests/external_interaction/test_zocalo_system.py b/tests/system_tests/external_interaction/test_zocalo_system.py index c01cfe37a..36fe6c421 100644 --- a/tests/system_tests/external_interaction/test_zocalo_system.py +++ b/tests/system_tests/external_interaction/test_zocalo_system.py @@ -6,8 +6,8 @@ from bluesky.run_engine import RunEngine from dodal.devices.zocalo import ZOCALO_READING_PLAN_NAME, ZocaloResults -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, +from hyperion.external_interaction.callbacks.common.callback_util import ( + gridscan_callbacks, ) from hyperion.parameters.constants import CONST from hyperion.parameters.plan_specific.gridscan_internal_params import ( @@ -60,7 +60,7 @@ def run_zocalo_with_dev_ispyb( ): async def inner(sample_name="", fallback=np.array([0, 0, 0])): dummy_params.hyperion_params.detector_params.prefix = sample_name - cbs = XrayCentreCallbackCollection() + cbs = [c() for c in gridscan_callbacks] ispyb = cbs.ispyb_handler ispyb.ispyb_config = dummy_ispyb_3d.ISPYB_CONFIG_PATH ispyb.active = True From 51dd0343aa2f5431724eb66ce5f074d61fb4ff4b Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 14 Mar 2024 12:09:08 +0000 Subject: [PATCH 08/23] #1199 replace old things with new --- .../panda_flyscan_xray_centre_plan.py | 6 +- .../callbacks/ispyb_callback_base.py | 4 +- .../callbacks/rotation/ispyb_callback.py | 4 +- .../callbacks/rotation/nexus_callback.py | 2 - .../callbacks/xray_centre/ispyb_callback.py | 2 - .../callbacks/xray_centre/nexus_callback.py | 2 - .../experiment_plans/test_fgs_plan.py | 32 ++++--- .../test_ispyb_dev_connection.py | 10 +- .../test_zocalo_system.py | 6 +- .../device_setup_plans/test_utils.py | 2 +- tests/unit_tests/experiment_plans/conftest.py | 10 +- .../test_flyscan_xray_centre_plan.py | 82 +++++++++------- .../test_panda_flyscan_xray_centre_plan.py | 69 ++++++------- .../callbacks/test_rotation_callbacks.py | 96 ++++++++++--------- .../callbacks/test_zocalo_handler.py | 8 +- .../test_xraycentre_callback_collection.py | 46 --------- 16 files changed, 170 insertions(+), 211 deletions(-) delete mode 100644 tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py diff --git a/src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py b/src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py index 550d97967..0fb476c32 100755 --- a/src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py @@ -40,8 +40,8 @@ set_aperture_for_bbox_size, wait_for_gridscan_valid, ) -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, +from hyperion.external_interaction.callbacks.common.callback_util import ( + create_gridscan_callbacks, ) from hyperion.log import LOGGER from hyperion.parameters import external_parameters @@ -313,7 +313,7 @@ def run_gridscan_and_move_and_tidy(fgs_composite, params): ) parameters = GridscanInternalParameters(**external_parameters.from_file()) - subscriptions = XrayCentreCallbackCollection() + subscriptions = create_gridscan_callbacks() context = setup_context(wait_for_connection=True) composite = create_devices(context) diff --git a/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py b/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py index 2db6ced70..30e9eda1b 100644 --- a/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py +++ b/src/hyperion/external_interaction/callbacks/ispyb_callback_base.py @@ -111,10 +111,10 @@ def activity_gated_event(self, doc: Event) -> Event: return self._tag_doc(doc) @abstractmethod - def update_deposition(self, params): + def update_deposition(self, params) -> IspybIds: pass - def activity_gated_stop(self, doc: RunStop) -> Optional[RunStop]: + def activity_gated_stop(self, doc: RunStop) -> RunStop: """Subclasses must check that they are recieving a stop document for the correct uid to use this method!""" assert isinstance( diff --git a/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py b/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py index cbc319030..17dfdc189 100644 --- a/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py +++ b/src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py @@ -42,8 +42,6 @@ class RotationISPyBCallback(BaseISPyBCallback): Or decorate a plan using bluesky.preprocessors.subs_decorator. See: https://blueskyproject.io/bluesky/callbacks.html#ways-to-invoke-callbacks - - Usually used as part of a RotationCallbackCollection. """ def __init__( @@ -151,7 +149,7 @@ def activity_gated_event(self, doc: Event): set_dcgid_tag(self.ispyb_ids.data_collection_group_id) return doc - def activity_gated_stop(self, doc: RunStop) -> None: + def activity_gated_stop(self, doc: RunStop) -> RunStop: if doc.get("run_start") == self.uid_to_finalize_on: self.uid_to_finalize_on = None return super().activity_gated_stop(doc) diff --git a/src/hyperion/external_interaction/callbacks/rotation/nexus_callback.py b/src/hyperion/external_interaction/callbacks/rotation/nexus_callback.py index 8895106d1..177dbd70d 100644 --- a/src/hyperion/external_interaction/callbacks/rotation/nexus_callback.py +++ b/src/hyperion/external_interaction/callbacks/rotation/nexus_callback.py @@ -31,8 +31,6 @@ class RotationNexusFileCallback(PlanReactiveCallback): Or decorate a plan using bluesky.preprocessors.subs_decorator. See: https://blueskyproject.io/bluesky/callbacks.html#ways-to-invoke-callbacks - - Usually used as part of a RotationCallbackCollection. """ def __init__(self) -> None: diff --git a/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py b/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py index b9c24f7ca..906a572ee 100644 --- a/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py +++ b/src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py @@ -50,8 +50,6 @@ class GridscanISPyBCallback(BaseISPyBCallback): Or decorate a plan using bluesky.preprocessors.subs_decorator. See: https://blueskyproject.io/bluesky/callbacks.html#ways-to-invoke-callbacks - - Usually used as part of an FGSCallbackCollection. """ def __init__( diff --git a/src/hyperion/external_interaction/callbacks/xray_centre/nexus_callback.py b/src/hyperion/external_interaction/callbacks/xray_centre/nexus_callback.py index 3c0bda7b4..196b46e7b 100644 --- a/src/hyperion/external_interaction/callbacks/xray_centre/nexus_callback.py +++ b/src/hyperion/external_interaction/callbacks/xray_centre/nexus_callback.py @@ -31,8 +31,6 @@ class GridscanNexusFileCallback(PlanReactiveCallback): Or decorate a plan using bluesky.preprocessors.subs_decorator. See: https://blueskyproject.io/bluesky/callbacks.html#ways-to-invoke-callbacks - - Usually used as part of an FGSCallbackCollection. """ def __init__(self) -> None: diff --git a/tests/system_tests/experiment_plans/test_fgs_plan.py b/tests/system_tests/experiment_plans/test_fgs_plan.py index 9d6db4722..5b30b24a6 100644 --- a/tests/system_tests/experiment_plans/test_fgs_plan.py +++ b/tests/system_tests/experiment_plans/test_fgs_plan.py @@ -1,5 +1,5 @@ import uuid -from typing import Callable +from typing import Callable, Tuple from unittest.mock import MagicMock, patch import bluesky.plan_stubs as bps @@ -28,8 +28,14 @@ FlyScanXRayCentreComposite, flyscan_xray_centre, ) -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, +from hyperion.external_interaction.callbacks.common.callback_util import ( + create_gridscan_callbacks, +) +from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import ( + GridscanISPyBCallback, +) +from hyperion.external_interaction.callbacks.xray_centre.nexus_callback import ( + GridscanNexusFileCallback, ) from hyperion.external_interaction.ispyb.ispyb_store import IspybIds from hyperion.parameters.constants import CONST @@ -60,8 +66,8 @@ def callbacks(params): with patch( "hyperion.external_interaction.callbacks.xray_centre.nexus_callback.NexusWriter" ): - callbacks = XrayCentreCallbackCollection() - callbacks.ispyb_handler.ispyb_config = CONST.SIM.DEV_ISPYB_DATABASE_CFG + callbacks = create_gridscan_callbacks() + callbacks[1].ispyb_config = CONST.SIM.DEV_ISPYB_DATABASE_CFG yield callbacks @@ -176,7 +182,7 @@ def test_xbpm_feedback_decorator( RE: RunEngine, fxc_composite: FlyScanXRayCentreComposite, params: GridscanInternalParameters, - callbacks: XrayCentreCallbackCollection, + callbacks: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], ): # This test is currently kind of more a unit test since we are faking XBPM feedback # with ophyd.sim, but it should continue to pass when we replace it with something @@ -216,13 +222,13 @@ def test_full_plan_tidies_at_end( fxc_composite: FlyScanXRayCentreComposite, params: GridscanInternalParameters, RE: RunEngine, - callbacks: XrayCentreCallbackCollection, + callbacks: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], ): RE(reset_positions(fxc_composite.smargon)) - callbacks.nexus_handler.nexus_writer_1 = MagicMock() - callbacks.nexus_handler.nexus_writer_2 = MagicMock() - callbacks.ispyb_handler.ispyb_ids = IspybIds( + callbacks[0].nexus_writer_1 = MagicMock() + callbacks[0].nexus_writer_2 = MagicMock() + callbacks[1].ispyb_ids = IspybIds( data_collection_ids=(0, 0), data_collection_group_id=0, grid_ids=(0,) ) [RE.subscribe(cb) for cb in callbacks] @@ -268,7 +274,7 @@ def test_GIVEN_scan_invalid_WHEN_plan_run_THEN_ispyb_entry_made_but_no_zocalo_en fxc_composite: FlyScanXRayCentreComposite, fetch_comment: Callable, params: GridscanInternalParameters, - callbacks: XrayCentreCallbackCollection, + callbacks: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], ): params.hyperion_params.detector_params.directory = "./tmp" params.hyperion_params.detector_params.prefix = str(uuid.uuid1()) @@ -282,9 +288,9 @@ def test_GIVEN_scan_invalid_WHEN_plan_run_THEN_ispyb_entry_made_but_no_zocalo_en with pytest.raises(WarningException): RE(flyscan_xray_centre(fxc_composite, params)) - ids = callbacks.ispyb_handler.ispyb_ids + ids = callbacks[1].ispyb_ids assert ids.data_collection_group_id is not None - dcid_used = callbacks.ispyb_handler.ispyb_ids.data_collection_ids[0] + dcid_used = callbacks[1].ispyb_ids.data_collection_ids[0] comment = fetch_comment(dcid_used) diff --git a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py index c3035fe68..c9c698df8 100644 --- a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py +++ b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py @@ -17,15 +17,15 @@ RotationScanComposite, rotation_scan, ) +from hyperion.external_interaction.callbacks.common.callback_util import ( + create_rotation_callbacks, +) from hyperion.external_interaction.callbacks.common.ispyb_mapping import ( GridScanInfo, populate_data_collection_group, populate_data_collection_position_info, populate_remaining_data_collection_info, ) -from hyperion.external_interaction.callbacks.rotation.callback_collection import ( - RotationCallbackCollection, -) from hyperion.external_interaction.callbacks.xray_centre.ispyb_mapping import ( construct_comment_for_gridscan, populate_data_collection_grid_info, @@ -343,7 +343,7 @@ def test_ispyb_deposition_in_rotation_plan( ) os.environ["ISPYB_CONFIG_PATH"] = CONST.SIM.DEV_ISPYB_DATABASE_CFG - callbacks = RotationCallbackCollection() + callbacks = create_rotation_callbacks() for cb in list(callbacks): RE.subscribe(cb) @@ -371,7 +371,7 @@ def test_ispyb_deposition_in_rotation_plan( ) ) - dcid = callbacks.ispyb_handler.ispyb_ids.data_collection_ids[0] + dcid = callbacks[1].ispyb_ids.data_collection_ids[0] assert dcid is not None comment = fetch_comment(dcid) assert comment == "Hyperion rotation scan" diff --git a/tests/system_tests/external_interaction/test_zocalo_system.py b/tests/system_tests/external_interaction/test_zocalo_system.py index 36fe6c421..8382194a2 100644 --- a/tests/system_tests/external_interaction/test_zocalo_system.py +++ b/tests/system_tests/external_interaction/test_zocalo_system.py @@ -7,7 +7,7 @@ from dodal.devices.zocalo import ZOCALO_READING_PLAN_NAME, ZocaloResults from hyperion.external_interaction.callbacks.common.callback_util import ( - gridscan_callbacks, + create_gridscan_callbacks, ) from hyperion.parameters.constants import CONST from hyperion.parameters.plan_specific.gridscan_internal_params import ( @@ -60,8 +60,8 @@ def run_zocalo_with_dev_ispyb( ): async def inner(sample_name="", fallback=np.array([0, 0, 0])): dummy_params.hyperion_params.detector_params.prefix = sample_name - cbs = [c() for c in gridscan_callbacks] - ispyb = cbs.ispyb_handler + cbs = create_gridscan_callbacks() + ispyb = cbs[1] ispyb.ispyb_config = dummy_ispyb_3d.ISPYB_CONFIG_PATH ispyb.active = True RE.subscribe(ispyb) diff --git a/tests/unit_tests/device_setup_plans/test_utils.py b/tests/unit_tests/device_setup_plans/test_utils.py index af79261e8..ffd7c6893 100644 --- a/tests/unit_tests/device_setup_plans/test_utils.py +++ b/tests/unit_tests/device_setup_plans/test_utils.py @@ -1,8 +1,8 @@ from unittest.mock import MagicMock import pytest -from bluesky import FailedStatus from bluesky import plan_stubs as bps +from bluesky.utils import FailedStatus from dodal.beamlines import i03 from ophyd.status import Status diff --git a/tests/unit_tests/experiment_plans/conftest.py b/tests/unit_tests/experiment_plans/conftest.py index 58de19bf5..6e181bace 100644 --- a/tests/unit_tests/experiment_plans/conftest.py +++ b/tests/unit_tests/experiment_plans/conftest.py @@ -11,8 +11,8 @@ from ophyd.sim import make_fake_device from ophyd_async.core.async_status import AsyncStatus -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, +from hyperion.external_interaction.callbacks.common.callback_util import ( + create_gridscan_callbacks, ) from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import ( GridscanISPyBCallback, @@ -139,14 +139,14 @@ def mock_subscriptions(test_fgs_params): ) ), ): - subscriptions = XrayCentreCallbackCollection() - subscriptions.ispyb_handler.ispyb = MagicMock(spec=StoreInIspyb) + subscriptions = create_gridscan_callbacks() + subscriptions[1].ispyb = MagicMock(spec=StoreInIspyb) start_doc = { "subplan_name": CONST.PLAN.GRIDSCAN_OUTER, "hyperion_internal_parameters": test_fgs_params.json(), CONST.TRIGGER.ZOCALO: CONST.PLAN.DO_FGS, } - subscriptions.ispyb_handler.activity_gated_start(start_doc) # type: ignore + subscriptions[1].activity_gated_start(start_doc) # type: ignore return subscriptions 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 index 9e8f34134..5d844f7ed 100644 --- a/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py @@ -1,5 +1,6 @@ import random import types +from typing import Tuple from unittest.mock import MagicMock, call, patch import bluesky.preprocessors as bpp @@ -32,15 +33,18 @@ run_gridscan_and_move, wait_for_gridscan_valid, ) +from hyperion.external_interaction.callbacks.common.callback_util import ( + create_gridscan_callbacks, +) from hyperion.external_interaction.callbacks.logging_callback import ( VerbosePlanExecutionLoggingCallback, ) -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, -) from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import ( GridscanISPyBCallback, ) +from hyperion.external_interaction.callbacks.xray_centre.nexus_callback import ( + GridscanNexusFileCallback, +) from hyperion.external_interaction.callbacks.zocalo_callback import ( ZocaloCallback, ) @@ -255,7 +259,9 @@ def test_results_adjusted_and_passed_to_move_xyz( move_aperture: MagicMock, fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], ): RE, _ = RE_with_subs RE.subscribe(VerbosePlanExecutionLoggingCallback()) @@ -362,15 +368,15 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( move_xyz: MagicMock, run_gridscan: MagicMock, move_aperture: MagicMock, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, ): RE, mock_subscriptions = RE_with_subs - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) RE( run_gridscan_and_move( fake_fgs_composite, @@ -395,14 +401,14 @@ def test_when_gridscan_finished_then_smargon_stub_offsets_are_set( move_xyz: MagicMock, run_gridscan: MagicMock, aperture_set: MagicMock, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): RE, mock_subscriptions = RE_with_subs - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) RE( run_gridscan_and_move( @@ -430,14 +436,14 @@ def test_when_gridscan_succeeds_ispyb_comment_appended_to( move_xyz: MagicMock, run_gridscan: MagicMock, aperture_set: MagicMock, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): RE, mock_subscriptions = RE_with_subs - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) RE.subscribe(VerbosePlanExecutionLoggingCallback()) @@ -447,9 +453,9 @@ def test_when_gridscan_succeeds_ispyb_comment_appended_to( test_fgs_params, ) ) - app_to_comment: MagicMock = ( - mock_subscriptions.ispyb_handler.ispyb.append_to_comment - ) # type:ignore + app_to_comment: MagicMock = mock_subscriptions[ + 1 + ].ispyb.append_to_comment # type:ignore app_to_comment.assert_called() call = app_to_comment.call_args_list[0] assert "Crystal 1: Strength 999999" in call.args[1] @@ -464,14 +470,14 @@ def test_when_gridscan_fails_ispyb_comment_appended_to( self, move_xyz: MagicMock, run_gridscan: MagicMock, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): RE, mock_subscriptions = RE_with_subs - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) mock_zocalo_trigger(fake_fgs_composite.zocalo, []) RE( run_gridscan_and_move( @@ -479,9 +485,9 @@ def test_when_gridscan_fails_ispyb_comment_appended_to( test_fgs_params, ) ) - app_to_comment: MagicMock = ( - mock_subscriptions.ispyb_handler.ispyb.append_to_comment - ) # type:ignore + app_to_comment: MagicMock = mock_subscriptions[ + 1 + ].ispyb.append_to_comment # type:ignore app_to_comment.assert_called() call = app_to_comment.call_args_list[0] assert "Zocalo found no crystals in this gridscan" in call.args[1] @@ -494,7 +500,9 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ self, move_xyz: MagicMock, mock_mv: MagicMock, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, done_status, @@ -511,9 +519,7 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ fake_fgs_composite.smargon.x.user_readback.sim_put(initial_x_y_z[0]) # type: ignore fake_fgs_composite.smargon.y.user_readback.sim_put(initial_x_y_z[1]) # type: ignore fake_fgs_composite.smargon.z.user_readback.sim_put(initial_x_y_z[2]) # type: ignore - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) mock_zocalo_trigger(fake_fgs_composite.zocalo, []) RE( run_gridscan_and_move( @@ -562,7 +568,9 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set( run_gridscan: MagicMock, fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], done_status, ): RE, mock_subscriptions = RE_with_subs @@ -570,9 +578,7 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set( return_value=done_status ) test_fgs_params.experiment_params.set_stub_offsets = False - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) RE.subscribe(VerbosePlanExecutionLoggingCallback()) @@ -650,7 +656,9 @@ def test_when_grid_scan_ran_then_eiger_disarmed_before_zocalo_end( mock_abs_set, fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], ): RE, mock_subscriptions = RE_with_subs # Put both mocks in a parent to easily capture order @@ -744,8 +752,8 @@ def test_kickoff_and_complete_gridscan_triggers_zocalo( ): id_1, id_2 = 100, 200 - cbs = XrayCentreCallbackCollection() - ispyb_cb = cbs.ispyb_handler + cbs = create_gridscan_callbacks() + ispyb_cb = cbs[1] ispyb_cb.active = True ispyb_cb.ispyb = MagicMock() ispyb_cb.params = MagicMock() 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 index a283cdd8c..0a5f51bda 100644 --- 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 @@ -1,5 +1,6 @@ import random import types +from typing import Tuple from unittest.mock import MagicMock, call, patch import bluesky.preprocessors as bpp @@ -37,12 +38,12 @@ from hyperion.external_interaction.callbacks.logging_callback import ( VerbosePlanExecutionLoggingCallback, ) -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, -) from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import ( GridscanISPyBCallback, ) +from hyperion.external_interaction.callbacks.xray_centre.nexus_callback import ( + GridscanNexusFileCallback, +) from hyperion.external_interaction.ispyb.ispyb_store import ( IspybIds, StoreInIspyb, @@ -219,7 +220,7 @@ def test_results_adjusted_and_passed_to_move_xyz( run_gridscan: MagicMock, move_aperture: MagicMock, fake_fgs_composite: FlyScanXRayCentreComposite, - mock_subscriptions: XrayCentreCallbackCollection, + mock_subscriptions: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], test_panda_fgs_params: PandAGridscanInternalParameters, RE: RunEngine, ): @@ -338,13 +339,11 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( run_gridscan: MagicMock, move_aperture: MagicMock, RE: RunEngine, - mock_subscriptions: XrayCentreCallbackCollection, + mock_subscriptions: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], fake_fgs_composite: FlyScanXRayCentreComposite, test_panda_fgs_params: PandAGridscanInternalParameters, ): - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_panda_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) RE( run_gridscan_and_move( fake_fgs_composite, @@ -377,13 +376,11 @@ def test_when_gridscan_finished_then_smargon_stub_offsets_are_set( run_gridscan: MagicMock, aperture_set: MagicMock, RE: RunEngine, - mock_subscriptions: XrayCentreCallbackCollection, + mock_subscriptions: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], test_panda_fgs_params: PandAGridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_panda_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) RE( run_gridscan_and_move( @@ -419,15 +416,13 @@ def test_when_gridscan_succeeds_ispyb_comment_appended_to( run_gridscan: MagicMock, aperture_set: MagicMock, RE: RunEngine, - mock_subscriptions: XrayCentreCallbackCollection, + mock_subscriptions: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], test_panda_fgs_params: PandAGridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_panda_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) - RE.subscribe(mock_subscriptions.ispyb_handler) + RE.subscribe(mock_subscriptions[1]) RE.subscribe(VerbosePlanExecutionLoggingCallback()) RE( @@ -436,9 +431,9 @@ def test_when_gridscan_succeeds_ispyb_comment_appended_to( test_panda_fgs_params, ) ) - app_to_comment: MagicMock = ( - mock_subscriptions.ispyb_handler.ispyb.append_to_comment - ) # type:ignore + app_to_comment: MagicMock = mock_subscriptions[ + 1 + ].ispyb.append_to_comment # type:ignore app_to_comment.assert_called() call = app_to_comment.call_args_list[0] assert "Crystal 1: Strength 999999" in call.args[1] @@ -461,24 +456,22 @@ def test_when_gridscan_fails_ispyb_comment_appended_to( move_xyz: MagicMock, run_gridscan: MagicMock, RE: RunEngine, - mock_subscriptions: XrayCentreCallbackCollection, + mock_subscriptions: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], test_panda_fgs_params: PandAGridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_panda_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) mock_zocalo_trigger(fake_fgs_composite.zocalo, []) - RE.subscribe(mock_subscriptions.ispyb_handler) + RE.subscribe(mock_subscriptions[1]) RE( run_gridscan_and_move( fake_fgs_composite, test_panda_fgs_params, ) ) - app_to_comment: MagicMock = ( - mock_subscriptions.ispyb_handler.ispyb.append_to_comment - ) # type:ignore + app_to_comment: MagicMock = mock_subscriptions[ + 1 + ].ispyb.append_to_comment # type:ignore app_to_comment.assert_called() call = app_to_comment.call_args_list[0] assert "Zocalo found no crystals in this gridscan" in call.args[1] @@ -499,7 +492,9 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ mock_setup_panda_for_flyscan: MagicMock, move_xyz: MagicMock, mock_mv: MagicMock, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], test_panda_fgs_params: PandAGridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, done_status, @@ -516,9 +511,7 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ fake_fgs_composite.smargon.x.user_readback.sim_put(initial_x_y_z[0]) # type: ignore fake_fgs_composite.smargon.y.user_readback.sim_put(initial_x_y_z[1]) # type: ignore fake_fgs_composite.smargon.z.user_readback.sim_put(initial_x_y_z[2]) # type: ignore - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_panda_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) mock_zocalo_trigger(fake_fgs_composite.zocalo, []) RE( run_gridscan_and_move( @@ -579,7 +572,7 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set( setup_panda_for_flyscan: MagicMock, move_xyz: MagicMock, run_gridscan: MagicMock, - mock_subscriptions: XrayCentreCallbackCollection, + mock_subscriptions: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], fake_fgs_composite: FlyScanXRayCentreComposite, test_panda_fgs_params: PandAGridscanInternalParameters, RE: RunEngine, @@ -589,9 +582,7 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set( return_value=done_status ) test_panda_fgs_params.experiment_params.set_stub_offsets = False - run_generic_ispyb_handler_setup( - mock_subscriptions.ispyb_handler, test_panda_fgs_params - ) + run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) RE.subscribe(VerbosePlanExecutionLoggingCallback()) @@ -681,8 +672,10 @@ def test_when_grid_scan_ran_then_eiger_disarmed_before_zocalo_end( mock_abs_set, fake_fgs_composite: FlyScanXRayCentreComposite, test_panda_fgs_params: PandAGridscanInternalParameters, - mock_subscriptions: XrayCentreCallbackCollection, - RE_with_subs: tuple[RunEngine, XrayCentreCallbackCollection], + mock_subscriptions: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], + RE_with_subs: tuple[ + RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] + ], ): RE, mock_subscriptions = RE_with_subs # Put both mocks in a parent to easily capture order diff --git a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py index 093191fc0..c388305a3 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py +++ b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py @@ -1,5 +1,5 @@ import os -from typing import Callable, Sequence +from typing import Callable, Sequence, Tuple from unittest.mock import MagicMock, patch import bluesky.plan_stubs as bps @@ -19,21 +19,18 @@ read_hardware_for_nexus_writer, read_hardware_for_zocalo, ) +from hyperion.external_interaction.callbacks.common.callback_util import ( + create_rotation_callbacks, +) from hyperion.external_interaction.callbacks.plan_reactive_callback import ( PlanReactiveCallback, ) -from hyperion.external_interaction.callbacks.rotation.callback_collection import ( - RotationCallbackCollection, -) from hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( RotationISPyBCallback, ) from hyperion.external_interaction.callbacks.rotation.nexus_callback import ( RotationNexusFileCallback, ) -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, -) from hyperion.external_interaction.exceptions import ISPyBDepositionNotMade from hyperion.external_interaction.ispyb.data_model import ExperimentType, ScanDataInfo from hyperion.external_interaction.ispyb.ispyb_store import ( @@ -72,14 +69,17 @@ def test_main_start_doc(): } -def activate_callbacks(cbs: RotationCallbackCollection | XrayCentreCallbackCollection): - cbs.ispyb_handler.active = True - cbs.nexus_handler.active = True +def activate_callbacks(cbs: Tuple[RotationNexusFileCallback, RotationISPyBCallback]): + cbs[1].active = True + cbs[0].active = True def fake_rotation_scan( params: RotationInternalParameters, - subscriptions: RotationCallbackCollection | Sequence[PlanReactiveCallback], + subscriptions: ( + Tuple[RotationNexusFileCallback, RotationISPyBCallback] + | Sequence[PlanReactiveCallback] + ), after_open_do: Callable | None = None, after_main_do: Callable | None = None, ): @@ -127,11 +127,11 @@ def fake_main_plan(): @pytest.fixture def activated_mocked_cbs(): - cb = RotationCallbackCollection() - cb.ispyb_handler.emit_cb = MagicMock + cb = create_rotation_callbacks() + cb[1].emit_cb = MagicMock activate_callbacks(cb) - cb.nexus_handler.activity_gated_event = MagicMock(autospec=True) - cb.nexus_handler.activity_gated_start = MagicMock(autospec=True) + cb[0].activity_gated_event = MagicMock(autospec=True) + cb[0].activity_gated_start = MagicMock(autospec=True) return cb @@ -143,9 +143,9 @@ def test_nexus_handler_gets_documents_in_mock_plan( ispyb, RE: RunEngine, params: RotationInternalParameters, - activated_mocked_cbs: RotationCallbackCollection, + activated_mocked_cbs: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): - nexus_handler = activated_mocked_cbs.nexus_handler + nexus_handler = activated_mocked_cbs[0] RE(fake_rotation_scan(params, [nexus_handler])) params.hyperion_params.ispyb_params.transmission_fraction = 1.0 @@ -219,14 +219,14 @@ def test_zocalo_start_and_end_not_triggered_if_ispyb_ids_not_present( test_outer_start_doc, ): nexus_writer.return_value.full_filename = "test_full_filename" - cb = RotationCallbackCollection() + cb = create_rotation_callbacks() activate_callbacks(cb) - cb.ispyb_handler.ispyb = MagicMock(spec=StoreInIspyb) - cb.ispyb_handler.params = params + cb[1].ispyb = MagicMock(spec=StoreInIspyb) + cb[1].params = params with pytest.raises(ISPyBDepositionNotMade): RE(fake_rotation_scan(params, cb)) - cb.ispyb_handler.emit_cb.zocalo_interactor.run_start.assert_not_called() # type: ignore + cb[1].emit_cb.zocalo_interactor.run_start.assert_not_called() # type: ignore @patch( @@ -254,21 +254,25 @@ def test_ispyb_starts_on_opening_and_zocalo_on_main_so_ispyb_triggered_before_zo ispyb_store.return_value = mock_store_in_ispyb_instance nexus_writer.return_value.full_filename = "test_full_filename" - cb = RotationCallbackCollection() + cb = create_rotation_callbacks() activate_callbacks(cb) - cb.ispyb_handler.emit_cb.stop = MagicMock() # type: ignore + cb[1].emit_cb.stop = MagicMock() # type: ignore - def after_open_do(callbacks: RotationCallbackCollection): - callbacks.ispyb_handler.ispyb.begin_deposition.assert_called_once() # pyright: ignore - callbacks.ispyb_handler.ispyb.update_deposition.assert_not_called() # pyright: ignore + def after_open_do( + callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], + ): + callbacks[1].ispyb.begin_deposition.assert_called_once() # pyright: ignore + callbacks[1].ispyb.update_deposition.assert_not_called() # pyright: ignore - def after_main_do(callbacks: RotationCallbackCollection): - callbacks.ispyb_handler.ispyb.update_deposition.assert_called_once() # pyright: ignore - cb.ispyb_handler.emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore + def after_main_do( + callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], + ): + callbacks[1].ispyb.update_deposition.assert_called_once() # pyright: ignore + cb[1].emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore RE(fake_rotation_scan(params, cb, after_open_do, after_main_do)) - cb.ispyb_handler.emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore + cb[1].emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore @patch( @@ -278,25 +282,27 @@ def after_main_do(callbacks: RotationCallbackCollection): def test_ispyb_handler_grabs_uid_from_main_plan_and_not_first_start_doc( zocalo, RE: RunEngine, params: RotationInternalParameters, test_outer_start_doc ): - cb = RotationCallbackCollection() - cb.ispyb_handler.emit_cb = None + cb = create_rotation_callbacks() + cb[1].emit_cb = None activate_callbacks(cb) - cb.nexus_handler.activity_gated_event = MagicMock(autospec=True) - cb.nexus_handler.activity_gated_start = MagicMock(autospec=True) - cb.ispyb_handler.activity_gated_start = MagicMock( - autospec=True, side_effect=cb.ispyb_handler.activity_gated_start + cb[0].activity_gated_event = MagicMock(autospec=True) + cb[0].activity_gated_start = MagicMock(autospec=True) + cb[1].activity_gated_start = MagicMock( + autospec=True, side_effect=cb[1].activity_gated_start ) - def after_open_do(callbacks: RotationCallbackCollection): - callbacks.ispyb_handler.activity_gated_start.assert_called_once() - assert callbacks.ispyb_handler.uid_to_finalize_on is None + def after_open_do( + callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], + ): + callbacks[1].activity_gated_start.assert_called_once() + assert callbacks[1].uid_to_finalize_on is None - def after_main_do(callbacks: RotationCallbackCollection): - cb.ispyb_handler.ispyb_ids = IspybIds( - data_collection_ids=(0,), data_collection_group_id=0 - ) - assert callbacks.ispyb_handler.activity_gated_start.call_count == 2 - assert callbacks.ispyb_handler.uid_to_finalize_on is not None + def after_main_do( + callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], + ): + cb[1].ispyb_ids = IspybIds(data_collection_ids=(0,), data_collection_group_id=0) + assert callbacks[1].activity_gated_start.call_count == 2 + assert callbacks[1].uid_to_finalize_on is not None with patch( "hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb", diff --git a/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py b/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py index 7f32eb503..f2e7b2b61 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py +++ b/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py @@ -3,8 +3,8 @@ import pytest from dodal.devices.zocalo import ZocaloStartInfo -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, +from hyperion.external_interaction.callbacks.common.callback_util import ( + create_gridscan_callbacks, ) from hyperion.external_interaction.callbacks.zocalo_callback import ZocaloCallback from hyperion.external_interaction.exceptions import ISPyBDepositionNotMade @@ -94,8 +94,8 @@ def test_execution_of_do_fgs_triggers_zocalo_calls( mock_ids = IspybIds(data_collection_ids=dc_ids, data_collection_group_id=dcg_id) ispyb_store.return_value.mock_add_spec(StoreInIspyb) - callbacks = XrayCentreCallbackCollection() - ispyb_cb = callbacks.ispyb_handler + callbacks = create_gridscan_callbacks() + ispyb_cb = callbacks[1] ispyb_cb.active = True assert isinstance(zocalo_handler := ispyb_cb.emit_cb, ZocaloCallback) zocalo_handler._reset_state() diff --git a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py b/tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py deleted file mode 100644 index 9131e9dbe..000000000 --- a/tests/unit_tests/external_interaction/callbacks/xray_centre/test_xraycentre_callback_collection.py +++ /dev/null @@ -1,46 +0,0 @@ -from unittest.mock import MagicMock - -import bluesky.plan_stubs as bps -import bluesky.preprocessors as bpp -from bluesky.run_engine import RunEngine - -from hyperion.external_interaction.callbacks.xray_centre.callback_collection import ( - XrayCentreCallbackCollection, -) - - -def test_callback_collection_init(): - callbacks = XrayCentreCallbackCollection() - assert len(list(callbacks)) == 2 - - -def test_callback_collection_list(): - callbacks = XrayCentreCallbackCollection() - callback_list = list(callbacks) - assert len(callback_list) == 2 - assert callbacks.ispyb_handler in callback_list - assert callbacks.nexus_handler in callback_list - - -def test_subscribe_in_plan(): - callbacks = XrayCentreCallbackCollection() - document_event_mock = MagicMock() - callbacks.ispyb_handler.start = document_event_mock - callbacks.ispyb_handler.activity_gated_stop = document_event_mock - callbacks.nexus_handler.activity_gated_start = document_event_mock - callbacks.nexus_handler.stop = document_event_mock - - RE = RunEngine() - - @bpp.subs_decorator(callbacks.ispyb_handler) - def outer_plan(): - @bpp.set_run_key_decorator("inner_plan") - @bpp.run_decorator(md={"subplan_name": "inner_plan"}) - def inner_plan(): - yield from bps.sleep(0) - - yield from inner_plan() - - RE(outer_plan()) - - document_event_mock.assert_called() From 00254e4b66a1a04f4feae89ff1e2755cb05f8fa0 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 14 Mar 2024 12:12:17 +0000 Subject: [PATCH 09/23] #1199 tidy test --- tests/unit_tests/hyperion/test_main_system.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/unit_tests/hyperion/test_main_system.py b/tests/unit_tests/hyperion/test_main_system.py index 35642eda4..116639c33 100644 --- a/tests/unit_tests/hyperion/test_main_system.py +++ b/tests/unit_tests/hyperion/test_main_system.py @@ -29,9 +29,6 @@ ) from hyperion.exceptions import WarningException from hyperion.experiment_plans.experiment_registry import PLAN_REGISTRY -from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import ( - AbstractPlanCallbackCollection, -) from hyperion.log import LOGGER from hyperion.parameters.cli import parse_cli_args from hyperion.parameters.plan_specific.gridscan_internal_params import ( @@ -353,10 +350,9 @@ def test_blueskyrunner_uses_cli_args_correctly_for_callbacks( mock_params.hyperion_params.experiment_type = "test_experiment" mock_param_class = MagicMock() mock_param_class.from_json.return_value = mock_params - callback_class_mock = MagicMock( - spec=AbstractPlanCallbackCollection, + callbacks_mock = MagicMock( name="mock_callback_class", - return_value=["test_cb_1", "test_cb_2"], + return_value=("test_cb_1", "test_cb_2"), ) TEST_REGISTRY = { @@ -364,7 +360,7 @@ def test_blueskyrunner_uses_cli_args_correctly_for_callbacks( "setup": MagicMock(), "internal_param_type": mock_param_class, "experiment_param_type": MagicMock(), - "callback_collection_type": callback_class_mock, + "callback_collection_type": callbacks_mock, } } @@ -398,8 +394,8 @@ class MockCommand: devices={}, experiment="test_experiment", parameters={}, - callbacks=callback_class_mock, - ), + callbacks=callbacks_mock, + ), # type: ignore block=True, # type: ignore ) runner.shutdown() From fd8d70702f6bc35de58c2beed037468d8b644db2 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 14 Mar 2024 12:17:58 +0000 Subject: [PATCH 10/23] ignore pyright in test lines --- .../external_interaction/callbacks/test_rotation_callbacks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py index c388305a3..27d27ab58 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py +++ b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py @@ -294,14 +294,14 @@ def test_ispyb_handler_grabs_uid_from_main_plan_and_not_first_start_doc( def after_open_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): - callbacks[1].activity_gated_start.assert_called_once() + callbacks[1].activity_gated_start.assert_called_once() # type: ignore assert callbacks[1].uid_to_finalize_on is None def after_main_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): cb[1].ispyb_ids = IspybIds(data_collection_ids=(0,), data_collection_group_id=0) - assert callbacks[1].activity_gated_start.call_count == 2 + assert callbacks[1].activity_gated_start.call_count == 2 # type: ignore assert callbacks[1].uid_to_finalize_on is not None with patch( From 8ca1908cfc4714b9b8d25731432848a69316e372 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 14 Mar 2024 12:28:49 +0000 Subject: [PATCH 11/23] (#1231) Check repo after argparse on deploy script so that --help is a noop --- utility_scripts/deploy/deploy_hyperion.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/utility_scripts/deploy/deploy_hyperion.py b/utility_scripts/deploy/deploy_hyperion.py index 98561f340..848943ffe 100644 --- a/utility_scripts/deploy/deploy_hyperion.py +++ b/utility_scripts/deploy/deploy_hyperion.py @@ -51,10 +51,7 @@ def set_deploy_location(self, release_area): # Get the release directory based off the beamline and the latest hyperion version -def get_hyperion_release_dir_from_args(repo: repo) -> str: - if repo.name != "hyperion": - raise ValueError("This function should only be used with the hyperion repo") - +def get_hyperion_release_dir_from_args() -> str: parser = argparse.ArgumentParser() parser.add_argument( "beamline", @@ -72,13 +69,16 @@ def get_hyperion_release_dir_from_args(repo: repo) -> str: if __name__ == "__main__": + # Gives path to /bluesky + release_area = get_hyperion_release_dir_from_args() + hyperion_repo = repo( name="hyperion", repo_args=os.path.join(os.path.dirname(__file__), "../../.git"), ) - # Gives path to /bluesky - release_area = get_hyperion_release_dir_from_args(hyperion_repo) + if hyperion_repo.name != "hyperion": + raise ValueError("This function should only be used with the hyperion repo") release_area_version = os.path.join( release_area, f"hyperion_{hyperion_repo.latest_version_str}" From fa480a22310fa5a37d5ac6add701da9cb824e5f1 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 14 Mar 2024 14:02:26 +0000 Subject: [PATCH 12/23] (#1231) Fix relative path for dls_dev_env and move into top variable --- utility_scripts/deploy/deploy_hyperion.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/utility_scripts/deploy/deploy_hyperion.py b/utility_scripts/deploy/deploy_hyperion.py index 848943ffe..585f12ce3 100644 --- a/utility_scripts/deploy/deploy_hyperion.py +++ b/utility_scripts/deploy/deploy_hyperion.py @@ -72,9 +72,13 @@ def get_hyperion_release_dir_from_args() -> str: # Gives path to /bluesky release_area = get_hyperion_release_dir_from_args() + this_repo_top = os.path.abspath(os.path.join(os.path.dirname(__file__), "../..")) + + print(f"Repo top is {this_repo_top}") + hyperion_repo = repo( name="hyperion", - repo_args=os.path.join(os.path.dirname(__file__), "../../.git"), + repo_args=os.path.join(this_repo_top, ".git"), ) if hyperion_repo.name != "hyperion": @@ -88,7 +92,7 @@ def get_hyperion_release_dir_from_args() -> str: dodal_repo = repo( name="dodal", - repo_args=os.path.join(os.path.dirname(__file__), "../../../dodal/.git"), + repo_args=os.path.join(this_repo_top, "../dodal/.git"), ) dodal_repo.set_deploy_location(release_area_version) @@ -113,8 +117,9 @@ def get_hyperion_release_dir_from_args() -> str: print(f"Setting up environment in {hyperion_repo.deploy_location}") if hyperion_repo.name == "hyperion": + env_script = os.path.join(this_repo_top, "utility_scripts/dls_dev_env.sh") with Popen( - "./utility_scripts/dls_dev_env.sh", + env_script, stdout=PIPE, bufsize=1, universal_newlines=True, @@ -144,4 +149,4 @@ def get_hyperion_release_dir_from_args() -> str: print(f"New version moved to {latest_location}") print("To start this version run hyperion_restart from the beamline's GDA") else: - print("Quiting without latest version being updated") + print("Quitting without latest version being updated") From af5bd1913f6624fd6553b6fa29ad008b4da0252c Mon Sep 17 00:00:00 2001 From: David Perl <115003895+dperl-dls@users.noreply.github.com> Date: Thu, 14 Mar 2024 15:18:07 +0000 Subject: [PATCH 13/23] Update tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py Co-authored-by: Dominic Oram --- .../experiment_plans/test_flyscan_xray_centre_plan.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 index 5d844f7ed..90b34da03 100644 --- a/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py @@ -752,8 +752,7 @@ def test_kickoff_and_complete_gridscan_triggers_zocalo( ): id_1, id_2 = 100, 200 - cbs = create_gridscan_callbacks() - ispyb_cb = cbs[1] + _, ispyb_cb= create_gridscan_callbacks() ispyb_cb.active = True ispyb_cb.ispyb = MagicMock() ispyb_cb.params = MagicMock() From 38bc4e7db59c236a0cc5b3cebacb1027461c0340 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 14 Mar 2024 15:39:06 +0000 Subject: [PATCH 14/23] 1199 tidy tests and make typealias --- src/hyperion/__main__.py | 5 +- .../experiment_plans/test_fgs_plan.py | 17 ++-- .../test_ispyb_dev_connection.py | 19 ++--- .../callbacks/test_rotation_callbacks.py | 80 +++++++++++-------- .../callbacks/test_zocalo_handler.py | 3 +- 5 files changed, 64 insertions(+), 60 deletions(-) diff --git a/src/hyperion/__main__.py b/src/hyperion/__main__.py index 610f5aa20..aa9612fb6 100755 --- a/src/hyperion/__main__.py +++ b/src/hyperion/__main__.py @@ -38,6 +38,7 @@ from hyperion.utils.context import setup_context VERBOSE_EVENT_LOGGING: Optional[bool] = None +CallbackFactories = Callable[[], Tuple[CallbackBase, CallbackBase]] @dataclass @@ -46,7 +47,7 @@ class Command: devices: Optional[Any] = None experiment: Optional[Callable[[Any, Any], MsgGenerator]] = None parameters: Optional[InternalParameters] = None - callbacks: Optional[Callable[[], Tuple[CallbackBase, CallbackBase]]] = None + callbacks: Optional[CallbackFactories] = None @dataclass @@ -109,7 +110,7 @@ def start( experiment: Callable, parameters: InternalParameters, plan_name: str, - callbacks: Optional[Callable[[], Tuple[CallbackBase, CallbackBase]]], + callbacks: Optional[CallbackFactories], ) -> StatusAndMessage: LOGGER.info(f"Started with parameters: {parameters}") diff --git a/tests/system_tests/experiment_plans/test_fgs_plan.py b/tests/system_tests/experiment_plans/test_fgs_plan.py index 5b30b24a6..e1e0d47a0 100644 --- a/tests/system_tests/experiment_plans/test_fgs_plan.py +++ b/tests/system_tests/experiment_plans/test_fgs_plan.py @@ -66,8 +66,8 @@ def callbacks(params): with patch( "hyperion.external_interaction.callbacks.xray_centre.nexus_callback.NexusWriter" ): - callbacks = create_gridscan_callbacks() - callbacks[1].ispyb_config = CONST.SIM.DEV_ISPYB_DATABASE_CFG + _, ispyb_cb = create_gridscan_callbacks() + ispyb_cb.ispyb_config = CONST.SIM.DEV_ISPYB_DATABASE_CFG yield callbacks @@ -225,10 +225,10 @@ def test_full_plan_tidies_at_end( callbacks: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], ): RE(reset_positions(fxc_composite.smargon)) - - callbacks[0].nexus_writer_1 = MagicMock() - callbacks[0].nexus_writer_2 = MagicMock() - callbacks[1].ispyb_ids = IspybIds( + nexus_cb, ispyb_cb = callbacks + nexus_cb.nexus_writer_1 = MagicMock() + nexus_cb.nexus_writer_2 = MagicMock() + ispyb_cb.ispyb_ids = IspybIds( data_collection_ids=(0, 0), data_collection_group_id=0, grid_ids=(0,) ) [RE.subscribe(cb) for cb in callbacks] @@ -276,6 +276,7 @@ def test_GIVEN_scan_invalid_WHEN_plan_run_THEN_ispyb_entry_made_but_no_zocalo_en params: GridscanInternalParameters, callbacks: Tuple[GridscanNexusFileCallback, GridscanISPyBCallback], ): + _, ispyb_cb = callbacks params.hyperion_params.detector_params.directory = "./tmp" params.hyperion_params.detector_params.prefix = str(uuid.uuid1()) params.hyperion_params.ispyb_params.visit_path = "/dls/i03/data/2022/cm31105-5/" @@ -288,9 +289,9 @@ def test_GIVEN_scan_invalid_WHEN_plan_run_THEN_ispyb_entry_made_but_no_zocalo_en with pytest.raises(WarningException): RE(flyscan_xray_centre(fxc_composite, params)) - ids = callbacks[1].ispyb_ids + ids = ispyb_cb.ispyb_ids assert ids.data_collection_group_id is not None - dcid_used = callbacks[1].ispyb_ids.data_collection_ids[0] + dcid_used = ispyb_cb.ispyb_ids.data_collection_ids[0] comment = fetch_comment(dcid_used) diff --git a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py index c9c698df8..7effdd9c0 100644 --- a/tests/system_tests/external_interaction/test_ispyb_dev_connection.py +++ b/tests/system_tests/external_interaction/test_ispyb_dev_connection.py @@ -17,15 +17,15 @@ RotationScanComposite, rotation_scan, ) -from hyperion.external_interaction.callbacks.common.callback_util import ( - create_rotation_callbacks, -) from hyperion.external_interaction.callbacks.common.ispyb_mapping import ( GridScanInfo, populate_data_collection_group, populate_data_collection_position_info, populate_remaining_data_collection_info, ) +from hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, +) from hyperion.external_interaction.callbacks.xray_centre.ispyb_mapping import ( construct_comment_for_gridscan, populate_data_collection_grid_info, @@ -304,14 +304,8 @@ def generate_scan_data_infos( @pytest.mark.s03 @patch("bluesky.plan_stubs.wait") -@patch("hyperion.external_interaction.callbacks.rotation.nexus_callback.NexusWriter") -@patch( - "hyperion.external_interaction.callbacks.rotation.callback_collection.ZocaloCallback" -) def test_ispyb_deposition_in_rotation_plan( bps_wait, - nexus_writer, - zocalo_callback, fake_create_rotation_devices: RotationScanComposite, RE: RunEngine, test_rotation_params: RotationInternalParameters, @@ -343,9 +337,8 @@ def test_ispyb_deposition_in_rotation_plan( ) os.environ["ISPYB_CONFIG_PATH"] = CONST.SIM.DEV_ISPYB_DATABASE_CFG - callbacks = create_rotation_callbacks() - for cb in list(callbacks): - RE.subscribe(cb) + ispyb_cb = RotationISPyBCallback() + RE.subscribe(ispyb_cb) composite = RotationScanComposite( attenuator=attenuator, @@ -371,7 +364,7 @@ def test_ispyb_deposition_in_rotation_plan( ) ) - dcid = callbacks[1].ispyb_ids.data_collection_ids[0] + dcid = ispyb_cb.ispyb_ids.data_collection_ids[0] assert dcid is not None comment = fetch_comment(dcid) assert comment == "Hyperion rotation scan" diff --git a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py index 27d27ab58..e0da39682 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py +++ b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py @@ -127,12 +127,12 @@ def fake_main_plan(): @pytest.fixture def activated_mocked_cbs(): - cb = create_rotation_callbacks() - cb[1].emit_cb = MagicMock - activate_callbacks(cb) - cb[0].activity_gated_event = MagicMock(autospec=True) - cb[0].activity_gated_start = MagicMock(autospec=True) - return cb + nexus_callback, ispyb_callback = create_rotation_callbacks() + ispyb_callback.emit_cb = MagicMock + activate_callbacks((nexus_callback, ispyb_callback)) + nexus_callback.activity_gated_event = MagicMock(autospec=True) + nexus_callback.activity_gated_start = MagicMock(autospec=True) + return nexus_callback, ispyb_callback @patch( @@ -145,7 +145,7 @@ def test_nexus_handler_gets_documents_in_mock_plan( params: RotationInternalParameters, activated_mocked_cbs: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): - nexus_handler = activated_mocked_cbs[0] + nexus_handler, _ = activated_mocked_cbs RE(fake_rotation_scan(params, [nexus_handler])) params.hyperion_params.ispyb_params.transmission_fraction = 1.0 @@ -219,14 +219,14 @@ def test_zocalo_start_and_end_not_triggered_if_ispyb_ids_not_present( test_outer_start_doc, ): nexus_writer.return_value.full_filename = "test_full_filename" - cb = create_rotation_callbacks() - activate_callbacks(cb) + nexus_callback, ispyb_callback = create_rotation_callbacks() + activate_callbacks((nexus_callback, ispyb_callback)) - cb[1].ispyb = MagicMock(spec=StoreInIspyb) - cb[1].params = params + ispyb_callback.ispyb = MagicMock(spec=StoreInIspyb) + ispyb_callback.params = params with pytest.raises(ISPyBDepositionNotMade): - RE(fake_rotation_scan(params, cb)) - cb[1].emit_cb.zocalo_interactor.run_start.assert_not_called() # type: ignore + RE(fake_rotation_scan(params, (nexus_callback, ispyb_callback))) + ispyb_callback.emit_cb.zocalo_interactor.run_start.assert_not_called() # type: ignore @patch( @@ -254,9 +254,9 @@ def test_ispyb_starts_on_opening_and_zocalo_on_main_so_ispyb_triggered_before_zo ispyb_store.return_value = mock_store_in_ispyb_instance nexus_writer.return_value.full_filename = "test_full_filename" - cb = create_rotation_callbacks() - activate_callbacks(cb) - cb[1].emit_cb.stop = MagicMock() # type: ignore + nexus_callback, ispyb_callback = create_rotation_callbacks() + activate_callbacks((nexus_callback, ispyb_callback)) + ispyb_callback.emit_cb.stop = MagicMock() # type: ignore def after_open_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], @@ -268,11 +268,15 @@ def after_main_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): callbacks[1].ispyb.update_deposition.assert_called_once() # pyright: ignore - cb[1].emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore + ispyb_callback.emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore - RE(fake_rotation_scan(params, cb, after_open_do, after_main_do)) + RE( + fake_rotation_scan( + params, (nexus_callback, ispyb_callback), after_open_do, after_main_do + ) + ) - cb[1].emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore + ispyb_callback.emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore @patch( @@ -282,13 +286,13 @@ def after_main_do( def test_ispyb_handler_grabs_uid_from_main_plan_and_not_first_start_doc( zocalo, RE: RunEngine, params: RotationInternalParameters, test_outer_start_doc ): - cb = create_rotation_callbacks() - cb[1].emit_cb = None - activate_callbacks(cb) - cb[0].activity_gated_event = MagicMock(autospec=True) - cb[0].activity_gated_start = MagicMock(autospec=True) - cb[1].activity_gated_start = MagicMock( - autospec=True, side_effect=cb[1].activity_gated_start + (nexus_callback, ispyb_callback) = create_rotation_callbacks() + ispyb_callback.emit_cb = None + activate_callbacks((nexus_callback, ispyb_callback)) + nexus_callback.activity_gated_event = MagicMock(autospec=True) + nexus_callback.activity_gated_start = MagicMock(autospec=True) + ispyb_callback.activity_gated_start = MagicMock( + autospec=True, side_effect=ispyb_callback.activity_gated_start ) def after_open_do( @@ -300,7 +304,9 @@ def after_open_do( def after_main_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): - cb[1].ispyb_ids = IspybIds(data_collection_ids=(0,), data_collection_group_id=0) + ispyb_callback.ispyb_ids = IspybIds( + data_collection_ids=(0,), data_collection_group_id=0 + ) assert callbacks[1].activity_gated_start.call_count == 2 # type: ignore assert callbacks[1].uid_to_finalize_on is not None @@ -308,7 +314,11 @@ def after_main_do( "hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb", autospec=True, ): - RE(fake_rotation_scan(params, cb, after_open_do, after_main_do)) + RE( + fake_rotation_scan( + params, (nexus_callback, ispyb_callback), after_open_do, after_main_do + ) + ) ids = [ @@ -334,8 +344,8 @@ def test_ispyb_reuses_dcgid_on_same_sampleID( params: RotationInternalParameters, ispyb_ids, ): - cb = [RotationISPyBCallback()] - cb[0].active = True + ispyb_cb = RotationISPyBCallback() + ispyb_cb.active = True ispyb_ids = IspybIds(data_collection_group_id=23, data_collection_ids=(45,)) rotation_ispyb.return_value.begin_deposition.return_value = ispyb_ids @@ -355,7 +365,7 @@ def after_open_do(callbacks: list[RotationISPyBCallback]): def after_main_do(callbacks: list[RotationISPyBCallback]): assert callbacks[0].uid_to_finalize_on is not None - RE(fake_rotation_scan(params, cb, after_open_do, after_main_do)) + RE(fake_rotation_scan(params, [ispyb_cb], after_open_do, after_main_do)) begin_deposition_scan_data: ScanDataInfo = ( rotation_ispyb.return_value.begin_deposition.call_args.args[1] @@ -368,7 +378,7 @@ def after_main_do(callbacks: list[RotationISPyBCallback]): else: assert begin_deposition_scan_data.data_collection_info.parent_id is None - last_dcgid = cb[0].ispyb_ids.data_collection_group_id + last_dcgid = ispyb_cb.ispyb_ids.data_collection_group_id @patch( @@ -380,8 +390,8 @@ def test_ispyb_specifies_experiment_type_if_supplied( RE: RunEngine, params: RotationInternalParameters, ): - cb = [RotationISPyBCallback()] - cb[0].active = True + ispyb_cb = RotationISPyBCallback() + ispyb_cb.active = True params.hyperion_params.ispyb_params.ispyb_experiment_type = "Characterization" rotation_ispyb.return_value.begin_deposition.return_value = IspybIds( data_collection_group_id=23, data_collection_ids=(45,) @@ -389,7 +399,7 @@ def test_ispyb_specifies_experiment_type_if_supplied( params.hyperion_params.ispyb_params.sample_id = "abc" - RE(fake_rotation_scan(params, cb)) + RE(fake_rotation_scan(params, [ispyb_cb])) assert rotation_ispyb.call_args.args[1] == ExperimentType.CHARACTERIZATION diff --git a/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py b/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py index f2e7b2b61..893a7505f 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py +++ b/tests/unit_tests/external_interaction/callbacks/test_zocalo_handler.py @@ -94,8 +94,7 @@ def test_execution_of_do_fgs_triggers_zocalo_calls( mock_ids = IspybIds(data_collection_ids=dc_ids, data_collection_group_id=dcg_id) ispyb_store.return_value.mock_add_spec(StoreInIspyb) - callbacks = create_gridscan_callbacks() - ispyb_cb = callbacks[1] + _, ispyb_cb = create_gridscan_callbacks() ispyb_cb.active = True assert isinstance(zocalo_handler := ispyb_cb.emit_cb, ZocaloCallback) zocalo_handler._reset_state() From 7562aaa0104a097836ea2791a98723a90d117b5c Mon Sep 17 00:00:00 2001 From: David Perl <115003895+dperl-dls@users.noreply.github.com> Date: Thu, 14 Mar 2024 15:46:59 +0000 Subject: [PATCH 15/23] Update utility_scripts/deploy/deploy_hyperion.py Co-authored-by: Dominic Oram --- utility_scripts/deploy/deploy_hyperion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utility_scripts/deploy/deploy_hyperion.py b/utility_scripts/deploy/deploy_hyperion.py index 585f12ce3..2df491f4a 100644 --- a/utility_scripts/deploy/deploy_hyperion.py +++ b/utility_scripts/deploy/deploy_hyperion.py @@ -137,7 +137,7 @@ def get_hyperion_release_dir_from_args() -> str: """ ) # Creates symlinks: software/bluesky/hyperion_latest -> software/bluesky/hyperion_{version}/hyperion - # software/bluesky/hyperion -> software/bluesky/hyperion + # software/bluesky/hyperion -> software/bluesky/hyperion_latest if move_symlink == "y": latest_location = os.path.join(release_area, "hyperion_latest") live_location = os.path.join(release_area, "hyperion") From 206b1c8a40b985c525493f29181f8593c47be893 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 14 Mar 2024 17:09:37 +0000 Subject: [PATCH 16/23] #1231 check to update stable link as well --- utility_scripts/deploy/deploy_hyperion.py | 49 +++++++++++++++-------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/utility_scripts/deploy/deploy_hyperion.py b/utility_scripts/deploy/deploy_hyperion.py index 2df491f4a..30cdc80f5 100644 --- a/utility_scripts/deploy/deploy_hyperion.py +++ b/utility_scripts/deploy/deploy_hyperion.py @@ -1,6 +1,7 @@ import argparse import os from subprocess import PIPE, CalledProcessError, Popen +from uuid import uuid1 from git import Repo from packaging.version import Version @@ -63,7 +64,7 @@ def get_hyperion_release_dir_from_args() -> str: args = parser.parse_args() if args.beamline == "dev": print("Running as dev") - return "/tmp/hyperion_release_test/bluesky" + return "/scratch/30day_tmp/hyperion_release_test/bluesky" else: return f"/dls_sw/{args.beamline}/software/bluesky" @@ -117,13 +118,10 @@ def get_hyperion_release_dir_from_args() -> str: print(f"Setting up environment in {hyperion_repo.deploy_location}") if hyperion_repo.name == "hyperion": - env_script = os.path.join(this_repo_top, "utility_scripts/dls_dev_env.sh") - with Popen( - env_script, - stdout=PIPE, - bufsize=1, - universal_newlines=True, - ) as p: + env_script = os.path.join( + hyperion_repo.deploy_location, "utility_scripts/dls_dev_env.sh" + ) + with Popen(env_script, stdout=PIPE, bufsize=1, universal_newlines=True) as p: if p.stdout is not None: for line in p.stdout: print(line, end="") @@ -131,6 +129,14 @@ def get_hyperion_release_dir_from_args() -> str: if p.returncode != 0: raise CalledProcessError(p.returncode, p.args) + def create_symlink_by_tmp_and_rename(dirname, target, linkname): + tmp_name = str(uuid1()) + target_path = os.path.join(dirname, target) + linkname_path = os.path.join(dirname, linkname) + tmp_path = os.path.join(dirname, tmp_name) + os.symlink(target_path, tmp_path) + os.rename(tmp_path, linkname_path) + move_symlink = input( """Move symlink (y/n)? WARNING: this will affect the running version! Only do so if you have informed the beamline scientist and you're sure Hyperion is not running. @@ -139,14 +145,25 @@ def get_hyperion_release_dir_from_args() -> str: # Creates symlinks: software/bluesky/hyperion_latest -> software/bluesky/hyperion_{version}/hyperion # software/bluesky/hyperion -> software/bluesky/hyperion_latest if move_symlink == "y": - latest_location = os.path.join(release_area, "hyperion_latest") - live_location = os.path.join(release_area, "hyperion") - new_tmp_location = os.path.join(release_area, "tmp_art") - os.symlink(hyperion_repo.deploy_location, new_tmp_location) - os.rename(new_tmp_location, latest_location) - os.symlink(latest_location, new_tmp_location) - os.rename(new_tmp_location, live_location) - print(f"New version moved to {latest_location}") + old_live_location = os.path.relpath( + os.path.realpath(os.path.join(release_area, "hyperion")), release_area + ) + make_live_stable_symlink = input( + f"The last live deployment was {old_live_location}, do you want to set this as the stable version? (y/n)" + ) + if make_live_stable_symlink == "y": + create_symlink_by_tmp_and_rename( + release_area, old_live_location, "hyperion_stable" + ) + + relative_deploy_loc = os.path.join( + os.path.relpath(hyperion_repo.deploy_location, release_area) + ) + create_symlink_by_tmp_and_rename( + release_area, relative_deploy_loc, "hyperion_latest" + ) + create_symlink_by_tmp_and_rename(release_area, "hyperion_latest", "hyperion") + print(f"New version moved to {hyperion_repo.deploy_location}") print("To start this version run hyperion_restart from the beamline's GDA") else: print("Quitting without latest version being updated") From 15258595f30394957005feecbfad66fd0c5aa029 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 15 Mar 2024 10:27:39 +0000 Subject: [PATCH 17/23] (#1199) Use named variables in tests rather than indexing tuples --- .../external_interaction/test_zocalo_system.py | 11 +++++------ tests/unit_tests/experiment_plans/conftest.py | 8 ++++---- .../callbacks/test_rotation_callbacks.py | 14 +++++++------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/tests/system_tests/external_interaction/test_zocalo_system.py b/tests/system_tests/external_interaction/test_zocalo_system.py index 8382194a2..d4351eb68 100644 --- a/tests/system_tests/external_interaction/test_zocalo_system.py +++ b/tests/system_tests/external_interaction/test_zocalo_system.py @@ -60,11 +60,10 @@ def run_zocalo_with_dev_ispyb( ): async def inner(sample_name="", fallback=np.array([0, 0, 0])): dummy_params.hyperion_params.detector_params.prefix = sample_name - cbs = create_gridscan_callbacks() - ispyb = cbs[1] - ispyb.ispyb_config = dummy_ispyb_3d.ISPYB_CONFIG_PATH - ispyb.active = True - RE.subscribe(ispyb) + _, ispyb_callback = create_gridscan_callbacks() + ispyb_callback.ispyb_config = dummy_ispyb_3d.ISPYB_CONFIG_PATH + ispyb_callback.active = True + RE.subscribe(ispyb_callback) @bpp.set_run_key_decorator("testing123") def trigger_zocalo_after_fast_grid_scan(): @@ -92,7 +91,7 @@ def inner_plan(): else: centre = centre[0] - return ispyb, ispyb.emit_cb, centre + return ispyb_callback, ispyb_callback.emit_cb, centre return inner diff --git a/tests/unit_tests/experiment_plans/conftest.py b/tests/unit_tests/experiment_plans/conftest.py index 6e181bace..a71e70ebc 100644 --- a/tests/unit_tests/experiment_plans/conftest.py +++ b/tests/unit_tests/experiment_plans/conftest.py @@ -139,16 +139,16 @@ def mock_subscriptions(test_fgs_params): ) ), ): - subscriptions = create_gridscan_callbacks() - subscriptions[1].ispyb = MagicMock(spec=StoreInIspyb) + nexus_callback, ispyb_callback = create_gridscan_callbacks() + ispyb_callback.ispyb = MagicMock(spec=StoreInIspyb) start_doc = { "subplan_name": CONST.PLAN.GRIDSCAN_OUTER, "hyperion_internal_parameters": test_fgs_params.json(), CONST.TRIGGER.ZOCALO: CONST.PLAN.DO_FGS, } - subscriptions[1].activity_gated_start(start_doc) # type: ignore + ispyb_callback.activity_gated_start(start_doc) # type: ignore - return subscriptions + return (nexus_callback, ispyb_callback) def fake_read(obj, initial_positions, _): diff --git a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py index e0da39682..2e9bcd389 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py +++ b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py @@ -261,13 +261,13 @@ def test_ispyb_starts_on_opening_and_zocalo_on_main_so_ispyb_triggered_before_zo def after_open_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): - callbacks[1].ispyb.begin_deposition.assert_called_once() # pyright: ignore - callbacks[1].ispyb.update_deposition.assert_not_called() # pyright: ignore + ispyb_callback.ispyb.begin_deposition.assert_called_once() # pyright: ignore + ispyb_callback.ispyb.update_deposition.assert_not_called() # pyright: ignore def after_main_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): - callbacks[1].ispyb.update_deposition.assert_called_once() # pyright: ignore + ispyb_callback.ispyb.update_deposition.assert_called_once() # pyright: ignore ispyb_callback.emit_cb.zocalo_interactor.run_start.assert_called_once() # type: ignore RE( @@ -298,8 +298,8 @@ def test_ispyb_handler_grabs_uid_from_main_plan_and_not_first_start_doc( def after_open_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], ): - callbacks[1].activity_gated_start.assert_called_once() # type: ignore - assert callbacks[1].uid_to_finalize_on is None + ispyb_callback.activity_gated_start.assert_called_once() # type: ignore + assert ispyb_callback.uid_to_finalize_on is None def after_main_do( callbacks: Tuple[RotationNexusFileCallback, RotationISPyBCallback], @@ -307,8 +307,8 @@ def after_main_do( ispyb_callback.ispyb_ids = IspybIds( data_collection_ids=(0,), data_collection_group_id=0 ) - assert callbacks[1].activity_gated_start.call_count == 2 # type: ignore - assert callbacks[1].uid_to_finalize_on is not None + assert ispyb_callback.activity_gated_start.call_count == 2 # type: ignore + assert ispyb_callback.uid_to_finalize_on is not None with patch( "hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb", From 1d2abdae3a759e024b6dc58229e3ccd2faed9afd Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 15 Mar 2024 10:29:51 +0000 Subject: [PATCH 18/23] (#1199) Move type def for callback factory to make it more widely used --- src/hyperion/__main__.py | 3 +-- src/hyperion/experiment_plans/experiment_registry.py | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hyperion/__main__.py b/src/hyperion/__main__.py index aa9612fb6..907c80e90 100755 --- a/src/hyperion/__main__.py +++ b/src/hyperion/__main__.py @@ -6,7 +6,6 @@ from typing import Any, Callable, Optional, Tuple from blueapi.core import BlueskyContext, MsgGenerator -from bluesky.callbacks import CallbackBase from bluesky.callbacks.zmq import Publisher from bluesky.run_engine import RunEngine from flask import Flask, request @@ -16,6 +15,7 @@ from hyperion.exceptions import WarningException from hyperion.experiment_plans.experiment_registry import ( PLAN_REGISTRY, + CallbackFactories, PlanNotFound, ) from hyperion.external_interaction.callbacks.__main__ import ( @@ -38,7 +38,6 @@ from hyperion.utils.context import setup_context VERBOSE_EVENT_LOGGING: Optional[bool] = None -CallbackFactories = Callable[[], Tuple[CallbackBase, CallbackBase]] @dataclass diff --git a/src/hyperion/experiment_plans/experiment_registry.py b/src/hyperion/experiment_plans/experiment_registry.py index 981f38e49..8ed1fa226 100644 --- a/src/hyperion/experiment_plans/experiment_registry.py +++ b/src/hyperion/experiment_plans/experiment_registry.py @@ -42,6 +42,8 @@ WaitForRobotLoadThenCentreParams, ) +CallbackFactories = Callable[[], Tuple[CallbackBase, CallbackBase]] + def not_implemented(): raise NotImplementedError @@ -62,7 +64,7 @@ class ExperimentRegistryEntry(TypedDict): | PandAGridscanInternalParameters ] experiment_param_type: type[AbstractExperimentParameterBase] - callback_factories: Callable[[], Tuple[CallbackBase, CallbackBase]] + callback_factories: CallbackFactories EXPERIMENT_TYPES = Union[GridScanParams, RotationScanParams] From 78595d605f46a539f4e0872abaf1034c947cf89d Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 18 Mar 2024 14:36:26 +0000 Subject: [PATCH 19/23] #1199 move typealias to more generic place and improve name --- src/hyperion/__main__.py | 9 +++++---- .../experiment_plans/experiment_registry.py | 18 +++++++++--------- .../callbacks/common/callback_util.py | 6 +++++- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/hyperion/__main__.py b/src/hyperion/__main__.py index aa9612fb6..70a3cb84f 100755 --- a/src/hyperion/__main__.py +++ b/src/hyperion/__main__.py @@ -6,7 +6,6 @@ from typing import Any, Callable, Optional, Tuple from blueapi.core import BlueskyContext, MsgGenerator -from bluesky.callbacks import CallbackBase from bluesky.callbacks.zmq import Publisher from bluesky.run_engine import RunEngine from flask import Flask, request @@ -24,6 +23,9 @@ from hyperion.external_interaction.callbacks.aperture_change_callback import ( ApertureChangeCallback, ) +from hyperion.external_interaction.callbacks.common.callback_util import ( + CallbacksFactory, +) from hyperion.external_interaction.callbacks.log_uid_tag_callback import ( LogUidTaggingCallback, ) @@ -38,7 +40,6 @@ from hyperion.utils.context import setup_context VERBOSE_EVENT_LOGGING: Optional[bool] = None -CallbackFactories = Callable[[], Tuple[CallbackBase, CallbackBase]] @dataclass @@ -47,7 +48,7 @@ class Command: devices: Optional[Any] = None experiment: Optional[Callable[[Any, Any], MsgGenerator]] = None parameters: Optional[InternalParameters] = None - callbacks: Optional[CallbackFactories] = None + callbacks: Optional[CallbacksFactory] = None @dataclass @@ -110,7 +111,7 @@ def start( experiment: Callable, parameters: InternalParameters, plan_name: str, - callbacks: Optional[CallbackFactories], + callbacks: Optional[CallbacksFactory], ) -> StatusAndMessage: LOGGER.info(f"Started with parameters: {parameters}") diff --git a/src/hyperion/experiment_plans/experiment_registry.py b/src/hyperion/experiment_plans/experiment_registry.py index 981f38e49..94bb672ff 100644 --- a/src/hyperion/experiment_plans/experiment_registry.py +++ b/src/hyperion/experiment_plans/experiment_registry.py @@ -1,8 +1,7 @@ from __future__ import annotations -from typing import Callable, Tuple, TypedDict, Union +from typing import Callable, TypedDict, Union -from bluesky.callbacks import CallbackBase from dodal.devices.fast_grid_scan import GridScanParams from dodal.devices.panda_fast_grid_scan import PandAGridScanParams from dodal.parameters.experiment_parameter_base import AbstractExperimentParameterBase @@ -16,6 +15,7 @@ wait_for_robot_load_then_centre_plan, ) from hyperion.external_interaction.callbacks.common.callback_util import ( + CallbacksFactory, create_gridscan_callbacks, create_rotation_callbacks, ) @@ -62,7 +62,7 @@ class ExperimentRegistryEntry(TypedDict): | PandAGridscanInternalParameters ] experiment_param_type: type[AbstractExperimentParameterBase] - callback_factories: Callable[[], Tuple[CallbackBase, CallbackBase]] + callbacks_factory: CallbacksFactory EXPERIMENT_TYPES = Union[GridScanParams, RotationScanParams] @@ -71,37 +71,37 @@ class ExperimentRegistryEntry(TypedDict): "setup": panda_flyscan_xray_centre_plan.create_devices, "internal_param_type": PandAGridscanInternalParameters, "experiment_param_type": PandAGridScanParams, - "callback_factories": create_gridscan_callbacks, + "callbacks_factory": create_gridscan_callbacks, }, "flyscan_xray_centre": { "setup": flyscan_xray_centre_plan.create_devices, "internal_param_type": GridscanInternalParameters, "experiment_param_type": GridScanParams, - "callback_factories": create_gridscan_callbacks, + "callbacks_factory": create_gridscan_callbacks, }, "grid_detect_then_xray_centre": { "setup": grid_detect_then_xray_centre_plan.create_devices, "internal_param_type": GridScanWithEdgeDetectInternalParameters, "experiment_param_type": GridScanWithEdgeDetectParams, - "callback_factories": create_gridscan_callbacks, + "callbacks_factory": create_gridscan_callbacks, }, "rotation_scan": { "setup": rotation_scan_plan.create_devices, "internal_param_type": RotationInternalParameters, "experiment_param_type": RotationScanParams, - "callback_factories": create_rotation_callbacks, + "callbacks_factory": create_rotation_callbacks, }, "pin_tip_centre_then_xray_centre": { "setup": pin_centre_then_xray_centre_plan.create_devices, "internal_param_type": PinCentreThenXrayCentreInternalParameters, "experiment_param_type": PinCentreThenXrayCentreParams, - "callback_factories": create_gridscan_callbacks, + "callbacks_factory": create_gridscan_callbacks, }, "wait_for_robot_load_then_centre": { "setup": wait_for_robot_load_then_centre_plan.create_devices, "internal_param_type": WaitForRobotLoadThenCentreInternalParameters, "experiment_param_type": WaitForRobotLoadThenCentreParams, - "callback_factories": create_gridscan_callbacks, + "callbacks_factory": create_gridscan_callbacks, }, } EXPERIMENT_NAMES = list(PLAN_REGISTRY.keys()) diff --git a/src/hyperion/external_interaction/callbacks/common/callback_util.py b/src/hyperion/external_interaction/callbacks/common/callback_util.py index 4ae604eb1..fabb803ec 100644 --- a/src/hyperion/external_interaction/callbacks/common/callback_util.py +++ b/src/hyperion/external_interaction/callbacks/common/callback_util.py @@ -1,4 +1,6 @@ -from typing import Tuple +from typing import Callable, Tuple + +from bluesky.callbacks import CallbackBase from hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( RotationISPyBCallback, @@ -14,6 +16,8 @@ ) from hyperion.external_interaction.callbacks.zocalo_callback import ZocaloCallback +CallbacksFactory = Callable[[], Tuple[CallbackBase, CallbackBase]] + def create_gridscan_callbacks() -> ( Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] From 22f2102f40f39993a8c07516de5b388e68f8d95d Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 18 Mar 2024 14:39:00 +0000 Subject: [PATCH 20/23] fix error from merge --- src/hyperion/experiment_plans/experiment_registry.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/hyperion/experiment_plans/experiment_registry.py b/src/hyperion/experiment_plans/experiment_registry.py index 564746e63..94bb672ff 100644 --- a/src/hyperion/experiment_plans/experiment_registry.py +++ b/src/hyperion/experiment_plans/experiment_registry.py @@ -42,8 +42,6 @@ WaitForRobotLoadThenCentreParams, ) -CallbackFactories = CallbacksFactory - def not_implemented(): raise NotImplementedError From 1581b99c93797dc3289982dc3a50d3c549763345 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 18 Mar 2024 14:46:53 +0000 Subject: [PATCH 21/23] #1199 get rid of more indexings --- .../test_flyscan_xray_centre_plan.py | 38 +++++++++---------- .../test_panda_flyscan_xray_centre_plan.py | 25 +++++++----- 2 files changed, 32 insertions(+), 31 deletions(-) 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 index 90b34da03..17e93cefd 100644 --- a/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py @@ -374,9 +374,9 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, ): - RE, mock_subscriptions = RE_with_subs + RE, (_, ispyb_cb) = RE_with_subs - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) + run_generic_ispyb_handler_setup(ispyb_cb, test_fgs_params) RE( run_gridscan_and_move( fake_fgs_composite, @@ -407,8 +407,8 @@ def test_when_gridscan_finished_then_smargon_stub_offsets_are_set( test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - RE, mock_subscriptions = RE_with_subs - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) + RE, (nexus_cb, ispyb_cb) = RE_with_subs + run_generic_ispyb_handler_setup(ispyb_cb, test_fgs_params) RE( run_gridscan_and_move( @@ -442,8 +442,8 @@ def test_when_gridscan_succeeds_ispyb_comment_appended_to( test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - RE, mock_subscriptions = RE_with_subs - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) + RE, (nexus_cb, ispyb_cb) = RE_with_subs + run_generic_ispyb_handler_setup(ispyb_cb, test_fgs_params) RE.subscribe(VerbosePlanExecutionLoggingCallback()) @@ -453,9 +453,7 @@ def test_when_gridscan_succeeds_ispyb_comment_appended_to( test_fgs_params, ) ) - app_to_comment: MagicMock = mock_subscriptions[ - 1 - ].ispyb.append_to_comment # type:ignore + app_to_comment: MagicMock = ispyb_cb.ispyb.append_to_comment # type:ignore app_to_comment.assert_called() call = app_to_comment.call_args_list[0] assert "Crystal 1: Strength 999999" in call.args[1] @@ -476,8 +474,8 @@ def test_when_gridscan_fails_ispyb_comment_appended_to( test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - RE, mock_subscriptions = RE_with_subs - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) + RE, (nexus_cb, ispyb_cb) = RE_with_subs + run_generic_ispyb_handler_setup(ispyb_cb, test_fgs_params) mock_zocalo_trigger(fake_fgs_composite.zocalo, []) RE( run_gridscan_and_move( @@ -485,9 +483,7 @@ def test_when_gridscan_fails_ispyb_comment_appended_to( test_fgs_params, ) ) - app_to_comment: MagicMock = mock_subscriptions[ - 1 - ].ispyb.append_to_comment # type:ignore + app_to_comment: MagicMock = ispyb_cb.ispyb.append_to_comment # type:ignore app_to_comment.assert_called() call = app_to_comment.call_args_list[0] assert "Zocalo found no crystals in this gridscan" in call.args[1] @@ -507,7 +503,7 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ fake_fgs_composite: FlyScanXRayCentreComposite, done_status, ): - RE, mock_subscriptions = RE_with_subs + RE, (nexus_cb, ispyb_cb) = RE_with_subs fake_fgs_composite.eiger.unstage = MagicMock(return_value=done_status) initial_x_y_z = np.array( [ @@ -519,7 +515,7 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ fake_fgs_composite.smargon.x.user_readback.sim_put(initial_x_y_z[0]) # type: ignore fake_fgs_composite.smargon.y.user_readback.sim_put(initial_x_y_z[1]) # type: ignore fake_fgs_composite.smargon.z.user_readback.sim_put(initial_x_y_z[2]) # type: ignore - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) + run_generic_ispyb_handler_setup(ispyb_cb, test_fgs_params) mock_zocalo_trigger(fake_fgs_composite.zocalo, []) RE( run_gridscan_and_move( @@ -573,12 +569,12 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set( ], done_status, ): - RE, mock_subscriptions = RE_with_subs + RE, (nexus_cb, ispyb_cb) = RE_with_subs fake_fgs_composite.aperture_scatterguard.set = MagicMock( return_value=done_status ) test_fgs_params.experiment_params.set_stub_offsets = False - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_fgs_params) + run_generic_ispyb_handler_setup(ispyb_cb, test_fgs_params) RE.subscribe(VerbosePlanExecutionLoggingCallback()) @@ -660,7 +656,7 @@ def test_when_grid_scan_ran_then_eiger_disarmed_before_zocalo_end( RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] ], ): - RE, mock_subscriptions = RE_with_subs + RE, (nexus_cb, ispyb_cb) = RE_with_subs # Put both mocks in a parent to easily capture order mock_parent = MagicMock() fake_fgs_composite.eiger.disarm_detector = mock_parent.disarm @@ -680,7 +676,7 @@ def test_when_grid_scan_ran_then_eiger_disarmed_before_zocalo_end( "hyperion.external_interaction.callbacks.zocalo_callback.ZocaloTrigger", lambda _: modified_interactor_mock(mock_parent.run_end), ): - [RE.subscribe(cb) for cb in list(mock_subscriptions)] + [RE.subscribe(cb) for cb in (nexus_cb, ispyb_cb)] RE(flyscan_xray_centre(fake_fgs_composite, test_fgs_params)) mock_parent.assert_has_calls([call.disarm(), call.run_end(0), call.run_end(0)]) @@ -752,7 +748,7 @@ def test_kickoff_and_complete_gridscan_triggers_zocalo( ): id_1, id_2 = 100, 200 - _, ispyb_cb= create_gridscan_callbacks() + _, ispyb_cb = create_gridscan_callbacks() ispyb_cb.active = True ispyb_cb.ispyb = MagicMock() ispyb_cb.params = MagicMock() 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 index 0a5f51bda..ed124a6bf 100644 --- 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 @@ -343,7 +343,8 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( fake_fgs_composite: FlyScanXRayCentreComposite, test_panda_fgs_params: PandAGridscanInternalParameters, ): - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) + _, ispyb_cb = mock_subscriptions + run_generic_ispyb_handler_setup(ispyb_cb, test_panda_fgs_params) RE( run_gridscan_and_move( fake_fgs_composite, @@ -380,7 +381,8 @@ def test_when_gridscan_finished_then_smargon_stub_offsets_are_set( test_panda_fgs_params: PandAGridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) + _, ispyb_cb = mock_subscriptions + run_generic_ispyb_handler_setup(ispyb_cb, test_panda_fgs_params) RE( run_gridscan_and_move( @@ -420,9 +422,10 @@ def test_when_gridscan_succeeds_ispyb_comment_appended_to( test_panda_fgs_params: PandAGridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) + _, ispyb_cb = mock_subscriptions + run_generic_ispyb_handler_setup(ispyb_cb, test_panda_fgs_params) - RE.subscribe(mock_subscriptions[1]) + RE.subscribe(ispyb_cb) RE.subscribe(VerbosePlanExecutionLoggingCallback()) RE( @@ -460,9 +463,10 @@ def test_when_gridscan_fails_ispyb_comment_appended_to( test_panda_fgs_params: PandAGridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) + _, ispyb_cb = mock_subscriptions + run_generic_ispyb_handler_setup(ispyb_cb, test_panda_fgs_params) mock_zocalo_trigger(fake_fgs_composite.zocalo, []) - RE.subscribe(mock_subscriptions[1]) + RE.subscribe(ispyb_cb) RE( run_gridscan_and_move( fake_fgs_composite, @@ -499,7 +503,7 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ fake_fgs_composite: FlyScanXRayCentreComposite, done_status, ): - RE, mock_subscriptions = RE_with_subs + RE, (nexus_cb, ispyb_cb) = RE_with_subs fake_fgs_composite.eiger.unstage = MagicMock(return_value=done_status) initial_x_y_z = np.array( [ @@ -511,7 +515,7 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ fake_fgs_composite.smargon.x.user_readback.sim_put(initial_x_y_z[0]) # type: ignore fake_fgs_composite.smargon.y.user_readback.sim_put(initial_x_y_z[1]) # type: ignore fake_fgs_composite.smargon.z.user_readback.sim_put(initial_x_y_z[2]) # type: ignore - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) + run_generic_ispyb_handler_setup(ispyb_cb, test_panda_fgs_params) mock_zocalo_trigger(fake_fgs_composite.zocalo, []) RE( run_gridscan_and_move( @@ -578,11 +582,12 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set( RE: RunEngine, done_status, ): + _, ispyb_cb = mock_subscriptions fake_fgs_composite.aperture_scatterguard.set = MagicMock( return_value=done_status ) test_panda_fgs_params.experiment_params.set_stub_offsets = False - run_generic_ispyb_handler_setup(mock_subscriptions[1], test_panda_fgs_params) + run_generic_ispyb_handler_setup(ispyb_cb, test_panda_fgs_params) RE.subscribe(VerbosePlanExecutionLoggingCallback()) @@ -677,7 +682,7 @@ def test_when_grid_scan_ran_then_eiger_disarmed_before_zocalo_end( RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] ], ): - RE, mock_subscriptions = RE_with_subs + RE, (nexus_cb, ispyb_cb) = RE_with_subs # Put both mocks in a parent to easily capture order mock_parent = MagicMock() fake_fgs_composite.eiger.disarm_detector = mock_parent.disarm From 60aad69cb1d551372c94638730307d695a835678 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 18 Mar 2024 14:51:10 +0000 Subject: [PATCH 22/23] #1199 another typealias to tidy tests --- .../test_flyscan_xray_centre_plan.py | 34 ++++++------------- 1 file changed, 10 insertions(+), 24 deletions(-) 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 index 17e93cefd..a30aa29be 100644 --- a/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py @@ -71,6 +71,8 @@ run_generic_ispyb_handler_setup, ) +ReWithSubs = tuple[RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback]] + @pytest.fixture def ispyb_plan(test_fgs_params): @@ -259,9 +261,7 @@ def test_results_adjusted_and_passed_to_move_xyz( move_aperture: MagicMock, fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, - RE_with_subs: tuple[ - RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] - ], + RE_with_subs: ReWithSubs, ): RE, _ = RE_with_subs RE.subscribe(VerbosePlanExecutionLoggingCallback()) @@ -368,9 +368,7 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( move_xyz: MagicMock, run_gridscan: MagicMock, move_aperture: MagicMock, - RE_with_subs: tuple[ - RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] - ], + RE_with_subs: ReWithSubs, fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, ): @@ -401,9 +399,7 @@ def test_when_gridscan_finished_then_smargon_stub_offsets_are_set( move_xyz: MagicMock, run_gridscan: MagicMock, aperture_set: MagicMock, - RE_with_subs: tuple[ - RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] - ], + RE_with_subs: ReWithSubs, test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): @@ -436,9 +432,7 @@ def test_when_gridscan_succeeds_ispyb_comment_appended_to( move_xyz: MagicMock, run_gridscan: MagicMock, aperture_set: MagicMock, - RE_with_subs: tuple[ - RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] - ], + RE_with_subs: ReWithSubs, test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): @@ -468,9 +462,7 @@ def test_when_gridscan_fails_ispyb_comment_appended_to( self, move_xyz: MagicMock, run_gridscan: MagicMock, - RE_with_subs: tuple[ - RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] - ], + RE_with_subs: ReWithSubs, test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, ): @@ -496,9 +488,7 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_ self, move_xyz: MagicMock, mock_mv: MagicMock, - RE_with_subs: tuple[ - RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] - ], + RE_with_subs: ReWithSubs, test_fgs_params: GridscanInternalParameters, fake_fgs_composite: FlyScanXRayCentreComposite, done_status, @@ -564,9 +554,7 @@ def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set( run_gridscan: MagicMock, fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, - RE_with_subs: tuple[ - RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] - ], + RE_with_subs: ReWithSubs, done_status, ): RE, (nexus_cb, ispyb_cb) = RE_with_subs @@ -652,9 +640,7 @@ def test_when_grid_scan_ran_then_eiger_disarmed_before_zocalo_end( mock_abs_set, fake_fgs_composite: FlyScanXRayCentreComposite, test_fgs_params: GridscanInternalParameters, - RE_with_subs: tuple[ - RunEngine, Tuple[GridscanNexusFileCallback, GridscanISPyBCallback] - ], + RE_with_subs: ReWithSubs, ): RE, (nexus_cb, ispyb_cb) = RE_with_subs # Put both mocks in a parent to easily capture order From 180683e8e5821ca2fc74b3b128debcca5b0d5163 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 18 Mar 2024 15:27:55 +0000 Subject: [PATCH 23/23] Update contributing.md --- contributing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributing.md b/contributing.md index d643c5607..3b0546275 100644 --- a/contributing.md +++ b/contributing.md @@ -8,7 +8,7 @@ General Workflow 1. An issue is created for the work. This issue should describe in as much detail as possible the work that needs to be done. Anyone is free to make a ticket and/or comment on one. 2. If a developer is going to do the work they assign themselves to the issue. 3. The developer makes a new branch with the format `issue_short_description` e.g. `122_create_a_contributing_file`. (External developers are also welcome to make forks) -4. The developer does the work on this branch, adding their work in small commits. Commit messages should be informative and prefixed with the issue number e.g. `#122: Added contributing file`. +4. The developer does the work on this branch, adding their work in small commits. Commit messages should be informative and prefixed with the issue number e.g. `(#122) Added contributing file`. 5. The developer submits a PR for the work. In the pull request should start with `Fixes #issue_num` e.g. `Fixes #122`, this will ensure the issue is automatically closed when the PR is merged. The developer should also add some background on how the reviewer might test the change. 6. If the developer has a particular person in mind to review the work they should assign that person to the PR as a reviewer. 7. The reviewer and developer go back and forth on the code until the reviewer approves it. (See [Reviewing Work](#reviewing-work))