From ceed3f752a92997b38aa1ca80734173829bcbd95 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 8 Sep 2026 12:33:10 +0000 Subject: [PATCH] Only remove attach_data_session_metadata --- .../devices/beamlines/i15_1/attenuator.py | 11 + src/dodal/plan_stubs/data_session.py | 62 --- src/dodal/plans/spec_path.py | 2 - src/dodal/plans/wrapped.py | 2 - .../beamlines/i15_1/test_attenuator.py | 27 ++ tests/plan_stubs/test_data_session.py | 43 -- tests/plans/conftest.py | 19 +- .../__init__.py | 0 .../test_verify_undulator_gap.py | 0 tests/preprocessors/__init__.py | 0 .../preprocessors/test_filesystem_metadata.py | 388 ------------------ 11 files changed, 41 insertions(+), 513 deletions(-) delete mode 100644 src/dodal/plan_stubs/data_session.py delete mode 100644 tests/plan_stubs/test_data_session.py rename tests/plans/{test_preprocessors => preprocessors}/__init__.py (100%) rename tests/plans/{test_preprocessors => preprocessors}/test_verify_undulator_gap.py (100%) delete mode 100644 tests/preprocessors/__init__.py delete mode 100644 tests/preprocessors/test_filesystem_metadata.py diff --git a/src/dodal/devices/beamlines/i15_1/attenuator.py b/src/dodal/devices/beamlines/i15_1/attenuator.py index 6d6f380e01d..a6e59df8ce4 100644 --- a/src/dodal/devices/beamlines/i15_1/attenuator.py +++ b/src/dodal/devices/beamlines/i15_1/attenuator.py @@ -17,6 +17,17 @@ class AttenuatorPositions(StrictEnum): TRANS_0_01 = "0.01%" TRANS_0_001 = "0.001%" + @classmethod + def from_trans_float(cls, trans: float) -> "AttenuatorPositions": + for position in cls: + if float(position.value.rstrip("%")) == trans: + return position + supported_transmissions = ", ".join(position.value for position in cls) + raise ValueError( + f"Unsupported transmission: {trans}. " + f"Supported transmissions are: {supported_transmissions}" + ) + class Attenuator(StandardReadable, Movable[AttenuatorPositions]): """A device to change the attenuation of the beam. diff --git a/src/dodal/plan_stubs/data_session.py b/src/dodal/plan_stubs/data_session.py deleted file mode 100644 index 62bbce3c785..00000000000 --- a/src/dodal/plan_stubs/data_session.py +++ /dev/null @@ -1,62 +0,0 @@ -import logging - -from bluesky import plan_stubs as bps -from bluesky import preprocessors as bpp -from bluesky.utils import MsgGenerator, make_decorator -from ophyd_async.core import PathProvider - -from dodal.common.beamlines.beamline_utils import get_path_provider -from dodal.common.types import UpdatingPathProvider - -DATA_SESSION = "data_session" -DATA_GROUPS = "data_groups" - - -def attach_data_session_metadata_wrapper( - plan: MsgGenerator, provider: PathProvider | None = None -) -> MsgGenerator: - """Attach data session metadata to the runs within a plan and make it correlate - with an ophyd-async PathProvider. - - This updates the path provider (which in turn makes a call to to a service - to figure out which scan number we are using for such a scan), and ensures the - start document contains the correct data session. - - Args: - plan (MsgGenerator): The plan to preprocess. - provider (PathProvider): The path provider that participating detectors are - aware of. - - Returns: - MsgGenerator: A plan. - - Yields: - Iterator[Msg]: Plan messages. - """ - try: - provider = provider or get_path_provider() - except NameError: - provider = None - - if isinstance(provider, UpdatingPathProvider): - yield from bps.wait_for([provider.update]) - ress = yield from bps.wait_for([provider.data_session]) - data_session = ress[0].result() - # https://github.com/DiamondLightSource/dodal/issues/452 - # As part of 452, write each dataCollection into their own folder, then can use resource_dir directly - yield from bpp.inject_md_wrapper(plan, md={DATA_SESSION: data_session}) - elif provider is None: - logging.warning( - f"There is no PathProvider set, {attach_data_session_metadata_wrapper.__name__} will have no effect" - ) - yield from plan - else: - logging.warning( - f"{provider} is not an UpdatingPathProvider, {attach_data_session_metadata_wrapper.__name__} will have no effect" - ) - yield from plan - - -attach_data_session_metadata_decorator = make_decorator( - attach_data_session_metadata_wrapper -) diff --git a/src/dodal/plans/spec_path.py b/src/dodal/plans/spec_path.py index 6f7f5a47f0a..a056300f9aa 100644 --- a/src/dodal/plans/spec_path.py +++ b/src/dodal/plans/spec_path.py @@ -9,10 +9,8 @@ from scanspec.specs import Spec from dodal.common import MsgGenerator -from dodal.plan_stubs.data_session import attach_data_session_metadata_decorator -@attach_data_session_metadata_decorator() @validate_call(config={"arbitrary_types_allowed": True}) def spec_scan( detectors: Annotated[ diff --git a/src/dodal/plans/wrapped.py b/src/dodal/plans/wrapped.py index 1ea01906db8..bdf0850b210 100644 --- a/src/dodal/plans/wrapped.py +++ b/src/dodal/plans/wrapped.py @@ -9,7 +9,6 @@ from pydantic import Field, NonNegativeFloat, validate_call from dodal.common import MsgGenerator -from dodal.plan_stubs.data_session import attach_data_session_metadata_decorator """This module wraps plan(s) from bluesky.plans until required handling for them is moved into bluesky or better handled in downstream services. @@ -26,7 +25,6 @@ """ -@attach_data_session_metadata_decorator() @validate_call(config={"arbitrary_types_allowed": True}) def count( detectors: Annotated[ diff --git a/tests/devices/beamlines/i15_1/test_attenuator.py b/tests/devices/beamlines/i15_1/test_attenuator.py index b83b95b03be..a893804e851 100644 --- a/tests/devices/beamlines/i15_1/test_attenuator.py +++ b/tests/devices/beamlines/i15_1/test_attenuator.py @@ -29,3 +29,30 @@ async def test_given_attenuator_device_in_position_then_can_read(attenuator): "attenuator-transmission": partial_reading("100%"), }, ) + + +@pytest.mark.parametrize( + "transmission, expected_position", + [ + (100, AttenuatorPositions.TRANS_100), + (50, AttenuatorPositions.TRANS_50), + (10, AttenuatorPositions.TRANS_10), + (1, AttenuatorPositions.TRANS_1), + (0.1, AttenuatorPositions.TRANS_0_1), + (0.01, AttenuatorPositions.TRANS_0_01), + (0.001, AttenuatorPositions.TRANS_0_001), + ], +) +def test_transmission_float_is_converted_to_position(transmission, expected_position): + assert AttenuatorPositions.from_trans_float(transmission) is expected_position + + +def test_unsupported_transmission_float_raises_value_error(): + with pytest.raises( + ValueError, + match=( + "Unsupported transmission: 2. " + "Supported transmissions are: 100%, 50%, 10%, 1%, 0.1%, 0.01%, 0.001%" + ), + ): + AttenuatorPositions.from_trans_float(2) diff --git a/tests/plan_stubs/test_data_session.py b/tests/plan_stubs/test_data_session.py deleted file mode 100644 index 1ef2da06d60..00000000000 --- a/tests/plan_stubs/test_data_session.py +++ /dev/null @@ -1,43 +0,0 @@ -from unittest.mock import Mock - -from bluesky.run_engine import RunEngine -from bluesky.utils import MsgGenerator - -from dodal.common.beamlines.beamline_utils import clear_path_provider -from dodal.plan_stubs.data_session import attach_data_session_metadata_wrapper - - -def test_attach_data_session_metadata_wrapper(caplog, run_engine: RunEngine): - def fake_plan() -> MsgGenerator[None]: - yield from [] - - path_provider = Mock() - plan = attach_data_session_metadata_wrapper( - plan=fake_plan(), provider=path_provider - ) - run_engine(plan) - - assert ( - f"{path_provider} is not an UpdatingPathProvider, {attach_data_session_metadata_wrapper.__name__} will have no effect" - in caplog.text - ) - - -def test_given_no_path_provider_when_running_clear_path_provider_then_noop(): - clear_path_provider() - - -def test_attach_data_session_metadata_wrapper_with_no_provider_is_noop( - caplog, run_engine: RunEngine -): - def fake_plan() -> MsgGenerator[None]: - yield from [] - - clear_path_provider() - plan = attach_data_session_metadata_wrapper(plan=fake_plan()) - run_engine(plan) - - assert ( - f"There is no PathProvider set, {attach_data_session_metadata_wrapper.__name__} will have no effect" - in caplog.text - ) diff --git a/tests/plans/conftest.py b/tests/plans/conftest.py index 400da3418bb..749c4befbbd 100644 --- a/tests/plans/conftest.py +++ b/tests/plans/conftest.py @@ -1,14 +1,9 @@ import asyncio from pathlib import Path, PurePath -from unittest.mock import patch import pytest from daq_config_server.client import ConfigClient -from ophyd_async.core import ( - PathProvider, - StandardDetector, - init_devices, -) +from ophyd_async.core import PathProvider, StandardDetector, init_devices from ophyd_async.sim import PatternGenerator, SimBlobDetector, SimMotor from dodal.devices.beamlines.i03.dcm import DCM @@ -40,7 +35,7 @@ async def mock_undulator_and_dcm( @pytest.fixture -def det(tmp_path: Path, path_provider) -> StandardDetector: +def det(tmp_path: Path, static_path_provider: PathProvider) -> StandardDetector: class DevNullPatternGenerator(PatternGenerator): def __init__(self, sleep=asyncio.sleep): super().__init__(sleep) @@ -63,7 +58,7 @@ def close_file(self): pattern_generator = DevNullPatternGenerator() with init_devices(mock=True): - det = SimBlobDetector(path_provider, pattern_generator) + det = SimBlobDetector(static_path_provider, pattern_generator) return det @@ -86,11 +81,3 @@ def z_axis() -> SimMotor: with init_devices(mock=True): z_axis = SimMotor() return z_axis - - -@pytest.fixture -def path_provider(static_path_provider: PathProvider): - # Prevents issue with leftover state from beamline tests - with patch("dodal.plan_stubs.data_session.get_path_provider") as mock: - mock.return_value = static_path_provider - yield static_path_provider diff --git a/tests/plans/test_preprocessors/__init__.py b/tests/plans/preprocessors/__init__.py similarity index 100% rename from tests/plans/test_preprocessors/__init__.py rename to tests/plans/preprocessors/__init__.py diff --git a/tests/plans/test_preprocessors/test_verify_undulator_gap.py b/tests/plans/preprocessors/test_verify_undulator_gap.py similarity index 100% rename from tests/plans/test_preprocessors/test_verify_undulator_gap.py rename to tests/plans/preprocessors/test_verify_undulator_gap.py diff --git a/tests/preprocessors/__init__.py b/tests/preprocessors/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tests/preprocessors/test_filesystem_metadata.py b/tests/preprocessors/test_filesystem_metadata.py deleted file mode 100644 index e95e5b42304..00000000000 --- a/tests/preprocessors/test_filesystem_metadata.py +++ /dev/null @@ -1,388 +0,0 @@ -from collections.abc import Callable, Mapping -from pathlib import Path -from typing import Any - -import bluesky.plan_stubs as bps -import bluesky.plans as bp -import pytest -from bluesky.preprocessors import ( - run_decorator, - run_wrapper, - set_run_key_decorator, - set_run_key_wrapper, - stage_wrapper, -) -from bluesky.protocols import HasName, Readable, Reading, Triggerable -from bluesky.run_engine import RunEngine -from bluesky.utils import MsgGenerator -from event_model.documents.event_descriptor import DataKey -from ophyd_async.core import AsyncStatus, PathProvider, init_devices -from pydantic import BaseModel - -from dodal.common.types import UpdatingPathProvider -from dodal.common.visit import ( - DataCollectionIdentifier, - DirectoryServiceClient, - LocalDirectoryServiceClient, - StaticVisitPathProvider, -) -from dodal.plan_stubs.data_session import ( - DATA_SESSION, - attach_data_session_metadata_wrapper, -) - - -class FakeDetector(Readable, HasName, Triggerable): - _name: str - _provider: PathProvider - - def __init__( - self, - name: str, - provider: PathProvider, - ) -> None: - self._name = name - self._provider = provider - - async def read(self) -> dict[str, Reading]: - return { - f"{self.name}_data": { - "value": "test", - "timestamp": 0.0, - }, - } - - async def describe(self) -> dict[str, DataKey]: - directory_info = self._provider(self.name) - source = str(directory_info.directory_path / f"{directory_info.filename}.h5") - return { - f"{self.name}_data": { - "dtype": "string", - "shape": [1], - "source": source, - } - } - - @AsyncStatus.wrap - async def trigger(self): - pass - - @property - def name(self) -> str: - return self._name - - @property - def parent(self) -> None: - return None - - -class MockDirectoryServiceClient(LocalDirectoryServiceClient): - def __init__(self): - self.fail = False - super().__init__() - - async def create_new_collection(self) -> DataCollectionIdentifier: - if self.fail: - raise ValueError() - - return await super().create_new_collection() - - async def get_current_collection(self) -> DataCollectionIdentifier: - if self.fail: - raise ValueError() - - return await super().get_current_collection() - - -class DataEvent(BaseModel): - name: str - doc: Any - - -@pytest.fixture -def client() -> DirectoryServiceClient: - return MockDirectoryServiceClient() - - -@pytest.fixture -def provider(client: DirectoryServiceClient, tmp_path: Path) -> UpdatingPathProvider: - return StaticVisitPathProvider("example", tmp_path, client=client) - - -@pytest.fixture(params=[1, 2]) -def detectors(request, provider: UpdatingPathProvider) -> list[Readable]: - number_of_detectors = request.param - with init_devices(mock=True): - dets: list[Readable] = [ - FakeDetector(name=f"det{i}", provider=provider) - for i in range(number_of_detectors) - ] - return dets - - -def simple_run(detectors: list[Readable]) -> MsgGenerator: - yield from bp.count(detectors) - - -def multi_run(detectors: list[Readable]) -> MsgGenerator: - yield from bp.count(detectors) - yield from bp.count(detectors) - - -def multi_nested_plan(detectors: list[Readable]) -> MsgGenerator: - yield from simple_run(detectors) - yield from simple_run(detectors) - - -def multi_run_single_stage(detectors: list[Readable]) -> MsgGenerator: - def stageless_count() -> MsgGenerator: - return (yield from bps.one_shot(detectors)) - - def inner_plan() -> MsgGenerator: - yield from run_wrapper(stageless_count()) - yield from run_wrapper(stageless_count()) - - yield from stage_wrapper(inner_plan(), detectors) - - -def multi_run_single_stage_multi_group( - detectors: list[Readable], -) -> MsgGenerator: - def stageless_count() -> MsgGenerator: - return (yield from bps.one_shot(detectors)) - - def inner_plan() -> MsgGenerator: - yield from run_wrapper(stageless_count(), md={DATA_SESSION: 1}) - yield from run_wrapper(stageless_count(), md={DATA_SESSION: 1}) - yield from run_wrapper(stageless_count(), md={DATA_SESSION: 2}) - yield from run_wrapper(stageless_count(), md={DATA_SESSION: 2}) - - yield from stage_wrapper(inner_plan(), detectors) - - -@run_decorator(md={DATA_SESSION: 12345}) -@set_run_key_decorator("outer") -def nested_run_with_metadata(detectors: list[Readable]) -> MsgGenerator: - yield from set_run_key_wrapper(bp.count(detectors), "inner") - yield from set_run_key_wrapper(bp.count(detectors), "inner") - - -@run_decorator() -@set_run_key_decorator("outer") -def nested_run_without_metadata( - detectors: list[Readable], -) -> MsgGenerator: - yield from set_run_key_wrapper(bp.count(detectors), "inner") - yield from set_run_key_wrapper(bp.count(detectors), "inner") - - -def test_simple_run_gets_scan_number( - run_engine: RunEngine, - detectors: list[Readable], - provider: UpdatingPathProvider, - tmp_path: Path, -) -> None: - docs = collect_docs( - run_engine, - simple_run(detectors), - provider, - ) - assert docs[0].name == "start" - assert docs[0].doc[DATA_SESSION] == "example-1" - assert_all_detectors_used_collection_numbers(tmp_path, docs, detectors, ["1"]) - - -@pytest.mark.parametrize("plan", [multi_run, multi_nested_plan]) -def test_multi_run_gets_scan_numbers( - run_engine: RunEngine, - detectors: list[Readable], - plan: Callable[[list[Readable]], MsgGenerator], - provider: UpdatingPathProvider, - tmp_path: Path, -) -> None: - """Test is here to demonstrate that multi run plans will overwrite files.""" - docs = collect_docs( - run_engine, - plan(detectors), - provider, - ) - start_docs = find_start_docs(docs) - assert len(start_docs) == 2 - assert start_docs[0].doc[DATA_SESSION] == "example-1" - assert start_docs[1].doc[DATA_SESSION] == "example-1" - assert_all_detectors_used_collection_numbers(tmp_path, docs, detectors, ["1", "1"]) - - -def test_multi_run_single_stage( - run_engine: RunEngine, - detectors: list[Readable], - provider: UpdatingPathProvider, - tmp_path: Path, -) -> None: - docs = collect_docs( - run_engine, - multi_run_single_stage(detectors), - provider, - ) - start_docs = find_start_docs(docs) - assert len(start_docs) == 2 - assert start_docs[0].doc[DATA_SESSION] == "example-1" - assert start_docs[1].doc[DATA_SESSION] == "example-1" - assert_all_detectors_used_collection_numbers( - tmp_path, - docs, - detectors, - [ - "1", - "1", - ], - ) - - -def test_multi_run_single_stage_multi_group( - run_engine: RunEngine, - detectors: list[Readable], - provider: UpdatingPathProvider, - tmp_path: Path, -) -> None: - docs = collect_docs( - run_engine, - multi_run_single_stage_multi_group(detectors), - provider, - ) - start_docs = find_start_docs(docs) - assert len(start_docs) == 4 - assert start_docs[0].doc[DATA_SESSION] == "example-1" - assert start_docs[1].doc[DATA_SESSION] == "example-1" - assert start_docs[2].doc[DATA_SESSION] == "example-1" - assert start_docs[3].doc[DATA_SESSION] == "example-1" - assert_all_detectors_used_collection_numbers( - tmp_path, - docs, - detectors, - ["1", "1", "1", "1"], - ) - - -def test_nested_run_with_metadata( - run_engine: RunEngine, - detectors: list[Readable], - provider: UpdatingPathProvider, - tmp_path: Path, -) -> None: - """Test is here to demonstrate that nested runs will be treated as a single run. - - That means detectors in such runs will overwrite files. - """ - docs = collect_docs( - run_engine, - nested_run_with_metadata(detectors), - provider, - ) - start_docs = find_start_docs(docs) - assert len(start_docs) == 3 - assert start_docs[0].doc[DATA_SESSION] == "example-1" - assert start_docs[1].doc[DATA_SESSION] == "example-1" - assert start_docs[2].doc[DATA_SESSION] == "example-1" - assert_all_detectors_used_collection_numbers(tmp_path, docs, detectors, ["1", "1"]) - - -def test_nested_run_without_metadata( - run_engine: RunEngine, - detectors: list[Readable], - provider: UpdatingPathProvider, - tmp_path: Path, -) -> None: - """Test is here to demonstrate that nested runs will be treated as a single run. - - That means detectors in such runs will overwrite files. - """ - docs = collect_docs( - run_engine, - nested_run_without_metadata(detectors), - provider, - ) - start_docs = find_start_docs(docs) - assert len(start_docs) == 3 - assert start_docs[0].doc[DATA_SESSION] == "example-1" - assert start_docs[1].doc[DATA_SESSION] == "example-1" - assert start_docs[2].doc[DATA_SESSION] == "example-1" - assert_all_detectors_used_collection_numbers(tmp_path, docs, detectors, ["1", "1"]) - - -def test_visit_path_provider_fails( - run_engine: RunEngine, - detectors: list[Readable], - provider: UpdatingPathProvider, - client: MockDirectoryServiceClient, -) -> None: - client.fail = True - with pytest.raises(ValueError): - collect_docs( - run_engine, - simple_run(detectors), - provider, - ) - - -def test_visit_path_provider_fails_after_one_sucess( - run_engine: RunEngine, - detectors: list[Readable], - provider: UpdatingPathProvider, - client: MockDirectoryServiceClient, -) -> None: - collect_docs( - run_engine, - simple_run(detectors), - provider, - ) - client.fail = True - with pytest.raises(ValueError): - collect_docs( - run_engine, - simple_run(detectors), - provider, - ) - - -def collect_docs( - run_engine: RunEngine, - plan: MsgGenerator, - provider: UpdatingPathProvider, -) -> list[DataEvent]: - events = [] - - def on_event(name: str, doc: Mapping[str, Any]) -> None: - events.append(DataEvent(name=name, doc=doc)) - - wrapped_plan = attach_data_session_metadata_wrapper(plan, provider) - run_engine(wrapped_plan, on_event) - return events - - -def assert_all_detectors_used_collection_numbers( - tmp_path: Path, - docs: list[DataEvent], - detectors: list[Readable], - data_collection_ids: list[str], -) -> None: - descriptors = find_descriptor_docs(docs) - assert len(descriptors) == len(data_collection_ids) - - for descriptor, data_collection_id in zip( - descriptors, data_collection_ids, strict=False - ): - for detector in detectors: - source = descriptor.doc.get("data_keys", {}).get(f"{detector.name}_data")[ - "source" - ] - expected_source = f"example-{data_collection_id}-{detector.name}.h5" - assert Path(source) == tmp_path / expected_source - - -def find_start_docs(docs: list[DataEvent]) -> list[DataEvent]: - return list(filter(lambda event: event.name == "start", docs)) - - -def find_descriptor_docs(docs: list[DataEvent]) -> list[DataEvent]: - return list(filter(lambda event: event.name == "descriptor", docs))