diff --git a/src/murfey/client/contexts/clem.py b/src/murfey/client/contexts/clem.py index 2af08afff..73f670751 100644 --- a/src/murfey/client/contexts/clem.py +++ b/src/murfey/client/contexts/clem.py @@ -10,7 +10,7 @@ from defusedxml.ElementTree import parse -from murfey.client.context import Context +from murfey.client.context import Context, _file_transferred_to, _get_source from murfey.client.instance_environment import MurfeyInstanceEnvironment from murfey.util.client import capture_post @@ -18,34 +18,6 @@ logger = logging.getLogger("murfey.client.contexts.clem") -def _file_transferred_to( - environment: MurfeyInstanceEnvironment, - source: Path, - file_path: Path, - rsync_basepath: Path, -): - """ - Returns the Path of the transferred file on the DLS file system. - """ - # Construct destination path - base_destination = rsync_basepath / Path(environment.default_destinations[source]) - # Add visit number to the path if it's not present in default destination - if environment.visit not in environment.default_destinations[source]: - base_destination = base_destination / environment.visit - destination = base_destination / file_path.relative_to(source) - return destination - - -def _get_source(file_path: Path, environment: MurfeyInstanceEnvironment): - """ - Returns the Path of the file on the client PC. - """ - for s in environment.sources: - if file_path.is_relative_to(s): - return s - return None - - def _get_image_elements(root: ET.Element) -> list[ET.Element]: """ Searches the XML metadata recursively to find the nodes tagged as "Element" that @@ -100,25 +72,23 @@ def post_transfer( transferred_file: Path, environment: MurfeyInstanceEnvironment | None = None, **kwargs, - ) -> bool: + ): super().post_transfer(transferred_file, environment=environment, **kwargs) + # Early exit if environment was not set + if not environment: + logger.warning("No environment was set") + return None + # Process files generated by "auto-save" acquisition mode # These include TIF/TIFF and XLIF files if transferred_file.suffix in (".tif", ".tiff", ".xlif"): logger.debug(f"File extension {transferred_file.suffix!r} detected") - - # Type checking to satisfy MyPy - if not environment: - logger.warning("No environment passed in") - return False - # Location of the file on the client PC source = _get_source(transferred_file, environment) - # Type checking to satisfy MyPy if not source: logger.warning(f"No source found for file {transferred_file}") - return False + return None # Get the file Path at the destination destination_file = _file_transferred_to( @@ -127,18 +97,13 @@ def post_transfer( file_path=transferred_file, rsync_basepath=Path(self._machine_config.get("rsync_basepath", "")), ) - if not destination_file: - logger.warning( - f"File {transferred_file.name!r} not found on the storage system" - ) - return False # Skip processing of binned "_pmd" image series if "_pmd_" in transferred_file.stem: logger.debug( f"File {transferred_file.name!r} belongs to the '_pmd_' series of binned images; skipping processing" ) - return True + return None # Process TIF/TIFF files if transferred_file.suffix in (".tif", ".tiff"): @@ -150,7 +115,7 @@ def post_transfer( logger.warning( f"File {transferred_file.name!r} is likely not part of the CLEM workflow" ) - return False + return None logger.debug( f"File {transferred_file.name!r} is part of a TIFF image series" ) @@ -179,13 +144,13 @@ def post_transfer( ) # Process XLIF files - if transferred_file.suffix == ".xlif": + elif transferred_file.suffix == ".xlif": # Skip processing of "_histo" histogram XLIF files if transferred_file.stem.endswith("_histo"): logger.debug( f"File {transferred_file.name!r} contains histogram metadata; skipping processing" ) - return True + return None # Skip processing of "IOManagerConfiguation.xlif" files # YES, the 'Configuation' typo IS part of the file name @@ -193,7 +158,7 @@ def post_transfer( logger.debug( f"File {transferred_file.name!r} is a Leica configuration file; skipping processing" ) - return True + return None logger.debug( f"File {transferred_file.name!r} contains metadata for an image series" @@ -242,12 +207,10 @@ def post_transfer( # .get(series_name, 0) returns 0 if no associated key is found if not len(self._tiff_series.get(series_name, [])): logger.debug(f"TIFF series {series_name!r} not yet loaded") - return True elif self._files_in_series.get(series_name, 0) == 0: logger.debug( f"Metadata for TIFF series {series_name!r} not yet processed" ) - return True elif len( self._tiff_series.get(series_name, []) ) >= self._files_in_series.get(series_name, 0): @@ -261,10 +224,8 @@ def post_transfer( "tiff_files": self._tiff_series[series_name][0:1], "series_metadata": self._series_metadata[series_name], } - post_result = self.process_tiff_series(tiff_dataset, environment) - if post_result is False: - return False - logger.info(f"Started preprocessing of TIFF series {series_name!r}") + if self.process_tiff_series(tiff_dataset, environment): + logger.info(f"Started preprocessing of TIFF series {series_name!r}") # Clean up memory after posting del self._tiff_series[series_name] @@ -274,18 +235,12 @@ def post_transfer( logger.debug(f"TIFF series {series_name!r} is still being processed") # Process LIF files - if transferred_file.suffix == ".lif": - # Type checking to satisfy MyPy - if not environment: - logger.warning("No environment passed in") - return False - + elif transferred_file.suffix == ".lif": # Location of the file on the client PC source = _get_source(transferred_file, environment) - # Type checking to satisfy MyPy if not source: logger.warning(f"No source found for file {transferred_file}") - return False + return None logger.debug( f"File {transferred_file.name!r} is a valid LIF file; starting processing" @@ -298,20 +253,13 @@ def post_transfer( file_path=transferred_file, rsync_basepath=Path(self._machine_config.get("rsync_basepath", "")), ) - if not destination_file: - logger.warning( - f"File {transferred_file.name!r} not found on the storage system" - ) - return False # Post URL to trigger job and convert LIF file into image stacks - post_result = self.process_lif_file(destination_file, environment) - if post_result is False: - return False - logger.info(f"Started preprocessing of {destination_file.name!r}") + if self.process_lif_file(destination_file, environment): + logger.info(f"Started preprocessing of {destination_file.name!r}") # Function has completed as expected - return True + return None def process_lif_file( self, diff --git a/src/murfey/client/contexts/fib.py b/src/murfey/client/contexts/fib.py index b5db2c13e..48964ef21 100644 --- a/src/murfey/client/contexts/fib.py +++ b/src/murfey/client/contexts/fib.py @@ -7,7 +7,7 @@ from pathlib import Path from typing import Callable, Type, TypeVar, cast -from murfey.client.context import Context +from murfey.client.context import Context, _file_transferred_to, _get_source from murfey.client.instance_environment import MurfeyInstanceEnvironment from murfey.util.client import capture_post from murfey.util.fib import get_slot_number, number_from_name @@ -200,34 +200,6 @@ def _get_project_name(file_path: Path): return None -def _get_source(file_path: Path, environment: MurfeyInstanceEnvironment) -> Path | None: - """ - Returns the Path of the file on the client PC. - """ - for s in environment.sources: - if file_path.is_relative_to(s): - return s - return None - - -def _file_transferred_to( - environment: MurfeyInstanceEnvironment, - source: Path, - file_path: Path, - rsync_basepath: Path, -) -> Path | None: - """ - Returns the Path of the transferred file on the DLS file system. - """ - # Construct destination path - base_destination = rsync_basepath / Path(environment.default_destinations[source]) - # Add visit number to the path if it's not present in default destination - if environment.visit not in environment.default_destinations[source]: - base_destination = base_destination / environment.visit - destination = base_destination / file_path.relative_to(source) - return destination - - @dataclass class FIBImage: images: list[Path] = field(default_factory=list) @@ -338,11 +310,6 @@ def post_transfer( file_path=transferred_file, rsync_basepath=Path(self._machine_config.get("rsync_basepath", "")), ) - if destination_file is None: - logger.warning( - f"Could not find destination file path for {transferred_file.name!r}" - ) - return None # Register image in database self._register_atlas(destination_file, environment) @@ -599,11 +566,6 @@ def _make_drift_correction_gif( file_path=file, rsync_basepath=Path(self._machine_config.get("rsync_basepath", "")), ) - if destination_file is None: - logger.warning( - f"Could not find destination file path for {file.name!r}" - ) - return else: destination_file = file diff --git a/tests/client/contexts/test_clem.py b/tests/client/contexts/test_clem.py new file mode 100644 index 000000000..24843f03f --- /dev/null +++ b/tests/client/contexts/test_clem.py @@ -0,0 +1,417 @@ +import textwrap +import xml.etree.ElementTree as ET +from pathlib import Path +from unittest import mock +from unittest.mock import MagicMock + +import pytest +from pytest_mock import MockerFixture + +from murfey.client.context import _file_transferred_to, _get_source +from murfey.client.contexts.clem import CLEMContext, _get_image_elements + +instrument_name = "clem" +session_id = 1 +visit_name = "cm12345-6" +project_name = "2025_06_30_10_00_00--grid_1001" +example_file_paths = [ + f"{project_name}{path}" + for path in [ + # TIFF files + "/TileScan 1/Position 1--Z00.tif", + "/TileScan 1/Position 1--C00.tif", + "/TileScan 1/Position 1--Stage00.tif", + "/TileScan 1/Position 1--Z00--C00.tif", + "/TileScan 1/Position 1--Stage00--C00.tif", + "/TileScan 1/Position 1--Stage00--Z00.tif", + "/TileScan 1/Position 1--Stage00--Z00--C00.tif", + "/TileScan 1/Position 1_ICC--Z00.tif", + "/TileScan 1/Position 1_Lng_LVCC--C00.tif", + "/TileScan 1/Position 1_Lng_SVCC--Stage00.tif", + "/TileScan 1/Position 1_ICC--Z00--C00.tif", + "/TileScan 1/Position 1_Lng_LVCC--Stage00--C00.tif", + "/TileScan 1/Position 1_Lng_SVCC--Stage00--Z00.tif", + "/TileScan 1/Metadata/Position 1.xlif", + "/Series001--Z00--C00.tif", + "/Series001--Stage00--C00.tif", + "/Series001--Stage00--Z00.tif", + "/Series001--Stage00--Z00--C00.tif", + "/Series001_ICC--Z00.tif", + "/Series001_Lng_LVCC--C00.tif", + "/Series001_Lng_SVCC--Stage00.tif", + "/Metadata/Series001_Lng_LVCC.xlif", + "/Image 1_ICC--Z00--C00.tif", + "/Image 1_Lng_LVCC--Stage00--C00.tif", + "/Image 1_Lng_SVCC--Stage00--Z00.tif", + "/Image 1--Z00.tif", + "/Image 1--C00.tif", + "/Image 1--Stage00.tif", + "/Image 1--Stage00--Z00--C00.tif", + "/Metadata/Image 1_ICC.xlif", + # LIF file + ".lif", + ] +] + + +def create_tiff_dataset( + visit_dir: Path, + series_path: str, + num_tiles: int, + num_frames: int, + num_channels: int, +) -> tuple[list[Path], Path, list[Path]]: + """ + Creates mock files mimicking the folder structure and naming pattern of actual + CLEM TIFF datasets. + """ + # Construct the TIFF files + tiff_files: list[Path] = [] + for c in range(num_channels or 1): + for t in range(num_tiles or 1): + for z in range(num_frames or 1): + # Construct name of file based on the presence of tiles, frames, and channels + file_name = series_path + if num_tiles > 1: + file_name += f"--Stage{str(t).zfill(2)}" + if num_frames > 1: + file_name += f"--Z{str(z).zfill(2)}" + if num_channels > 1: + file_name += f"--C{str(c).zfill(2)}" + file_name += ".tif" + file = visit_dir / "images" / file_name + tiff_files.append(file) + + # Construct the metadata file + file_name = f"{series_path}.xlif" + metadata_file = visit_dir / "images" / file_name + # Insert "Metadata" before the file name + metadata_file = metadata_file.parent / "Metadata" / metadata_file.name + + # Construct junk files + histo_file = visit_dir / "images" / f"{series_path}_histo.xlif" + histo_file = histo_file.parent / "Metadata" / histo_file.name + + io_file = ( + visit_dir / "images" / project_name / "Metadata" / "IOManagerConfiguation.xlif" + ) + + for file in [*tiff_files, metadata_file, histo_file, io_file]: + file.parent.mkdir(parents=True, exist_ok=True) + file.touch(exist_ok=True) + + return tiff_files, metadata_file, [histo_file, io_file] + + +@pytest.fixture +def visit_dir( + tmp_path: Path, +): + visit_dir = tmp_path / visit_name + visit_dir.mkdir(parents=True, exist_ok=True) + return visit_dir + + +def test_get_image_elements(): + # Create a mock XML Element to read + mock_xml_str = textwrap.dedent("""\ + + + + + + Dummy + + + + + + + Dummy + + + + + + + """) + mock_xml = ET.fromstring(mock_xml_str) + assert len(_get_image_elements(mock_xml)) == 2 + + +@pytest.mark.parametrize("file_path", example_file_paths) +def test_get_source( + tmp_path: Path, + visit_dir: Path, + file_path: str, +): + # Mock the MurfeyInstanceEnvironment + mock_environment = MagicMock() + mock_environment.sources = [ + visit_dir, + tmp_path / "another_dir", + ] + # Check that the correct source directory is found + assert _get_source(visit_dir / "images" / file_path, mock_environment) == visit_dir + + +@pytest.mark.parametrize("file_path", example_file_paths) +def test_file_transferred_to(tmp_path: Path, visit_dir: Path, file_path: str): + # Create the client-side file + file = visit_dir / "images" / file_path + + # Mock the environment + mock_environment = MagicMock() + mock_environment.default_destinations = {visit_dir: "current_year"} + mock_environment.visit = visit_name + + # Iterate across the FIB files to compare against + destination_dir = tmp_path / "clem" / "data" / "current_year" / visit_name + # Work out what the expected destination will be + assert _file_transferred_to( + environment=mock_environment, + source=visit_dir, + file_path=file, + rsync_basepath=tmp_path / "clem" / "data", + ) == destination_dir / file.relative_to(visit_dir) + + +@pytest.mark.parametrize( + "test_params", + ( # Has environment | Has source + (True, True), + (False, True), + (True, False), + ), +) +def test_post_transfer_lif_data( + mocker: MockerFixture, + tmp_path: Path, + visit_dir: Path, + test_params: tuple[bool, bool], +): + # Unpack test params + has_env, has_src = test_params + + # Create a mock LIF file and its destination path + rsync_basepath = tmp_path / "data" / "clem" + src = visit_dir / "images" / f"{project_name}.lif" + dst = rsync_basepath / "current_year" / src.relative_to(visit_dir.parent) + + # Mock the environment + mock_environment = ( + MagicMock(instrument_name=instrument_name, murfey_session=session_id) + if has_env + else None + ) + + # Mock '_get_source' + mock_get_source = mocker.patch( + "murfey.client.contexts.clem._get_source", + return_value=visit_dir if has_src else None, + ) + + # Mock '_file_transferred_to' + mock_file_transferred_to = mocker.patch( + "murfey.client.contexts.clem._file_transferred_to", return_value=dst + ) + + # Mock 'capture_post' + mock_capture_post = mocker.patch( + "murfey.client.contexts.clem.capture_post", return_value=True + ) + + # Initialise the CLEMContext + context = CLEMContext( + acquisition_software="leica", + basepath=tmp_path, + machine_config={"rsync_basepath": str(rsync_basepath)}, + token="dummy", + ) + # Run the function on the LIF file + context.post_transfer( + src, + environment=mock_environment, + ) + + # Check that the calls were made with the expected parameters + if not has_env: + mock_get_source.assert_not_called() + mock_file_transferred_to.assert_not_called() + mock_capture_post.assert_not_called() + else: + mock_get_source.assert_called_once_with(src, mock_environment) + if not has_src: + mock_file_transferred_to.assert_not_called() + mock_capture_post.assert_not_called() + else: + mock_file_transferred_to.assert_called_once_with( + environment=mock_environment, + source=visit_dir, + file_path=src, + rsync_basepath=rsync_basepath, + ) + mock_capture_post.assert_called_once_with( + base_url=mock.ANY, + router_name="workflow_clem.router", + function_name="process_raw_lifs", + token=context._token, + instrument_name=instrument_name, + session_id=session_id, + data={"lif_file": str(dst)}, + ) + + +@pytest.mark.parametrize( + "test_params", + ( # Has environment | Has source | Reverse order | Series path | Tiles | Channels | Frames + # Success cases + (True, True, True, "TileScan 1/Position 1", 4, 1, 1), + (True, True, False, "TileScan 1/Position 1_ICC", 1, 3, 1), + (True, True, True, "TileScan 1/Position 1_Lng_LVCC", 1, 1, 5), + (True, True, False, "TileScan 1/Position 1_Lng_SVCC", 4, 3, 1), + (True, True, True, "Image 1", 4, 1, 5), + (True, True, False, "Image 1_ICC", 1, 3, 5), + (True, True, True, "Image 1_Lng_LVCC", 4, 3, 5), + (True, True, False, "Image 1_Lng_SVCC", 4, 1, 1), + (True, True, True, "Series001", 1, 3, 1), + (True, True, False, "Series001_ICC", 1, 1, 5), + (True, True, True, "Series001_Lng_LVCC", 4, 3, 1), + (True, True, False, "Series001_Lng_SVCC", 4, 1, 5), + # Fail cases + (False, True, False, "TileScan 1/Position 1", 1, 3, 5), + (True, False, False, "TileScan 1/Position 1", 4, 3, 5), + ), +) +def test_post_transfer_tiff_data( + mocker: MockerFixture, + tmp_path: Path, + visit_dir: Path, + test_params: tuple[bool, bool, bool, str, int, int, int], +): + # Unpack test params + ( + has_env, + has_src, + reverse_files, + series_path, + num_tiles, + num_channels, + num_frames, + ) = test_params + + # Create a mock LIF file and its destination pathparent) + tiff_files, metadata, junk_files = create_tiff_dataset( + visit_dir, + series_path, + num_tiles=num_tiles, + num_frames=num_frames, + num_channels=num_channels, + ) + all_files = [*tiff_files, metadata, *junk_files] + if reverse_files: + all_files.reverse() + + # Create their destination + rsync_basepath = tmp_path / "data" / "clem" + dst_metadata = rsync_basepath / metadata.relative_to(visit_dir.parent) + dst_files = [ + rsync_basepath / file.relative_to(visit_dir.parent) for file in all_files + ] + + # Mock the environment + mock_environment = ( + MagicMock(instrument_name=instrument_name, murfey_session=session_id) + if has_env + else None + ) + + # Mock '_get_source' + mock_get_source = mocker.patch( + "murfey.client.contexts.clem._get_source", + return_value=visit_dir if has_src else None, + ) + + # Mock '_file_transferred_to' + mock_file_transferred_to = mocker.patch( + "murfey.client.contexts.clem._file_transferred_to", side_effect=dst_files + ) + + # Mock 'capture_post' + mock_capture_post = mocker.patch( + "murfey.client.contexts.clem.capture_post", return_value=True + ) + + # Mock 'parse' + mock_parse = mocker.patch("murfey.client.contexts.clem.parse") + mock_parse.getroot.return_value = MagicMock() + + # Mock '_get_image_elements' to return a mock XML + channel_block = ( + textwrap.dedent(""" + + """) + * num_channels + ) + dimension_block = "" + if num_frames > 1: + dimension_block += textwrap.dedent(f""" + + """) + if num_tiles > 1: + dimension_block += textwrap.dedent(f""" + + """) + mock_xml_string = f"""{channel_block}{dimension_block}""" + mock_get_image_elements = mocker.patch( + "murfey.client.contexts.clem._get_image_elements", + return_value=[ET.fromstring(mock_xml_string)], + ) + + # Initialise the CLEMContext + context = CLEMContext( + acquisition_software="leica", + basepath=tmp_path, + machine_config={"rsync_basepath": str(rsync_basepath)}, + token="dummy", + ) + # Run the function on the TIFF files + for file in all_files: + context.post_transfer( + file, + environment=mock_environment, + ) + + # Check that the calls were made with the expected parameters + if not has_env: + mock_get_source.assert_not_called() + mock_file_transferred_to.assert_not_called() + mock_get_image_elements.assert_not_called() + mock_capture_post.assert_not_called() + else: + for file in all_files: + mock_get_source.assert_any_call(file, mock_environment) + if not has_src: + mock_file_transferred_to.assert_not_called() + mock_capture_post.assert_not_called() + else: + for file in all_files: + mock_file_transferred_to.assert_any_call( + environment=mock_environment, + source=visit_dir, + file_path=file, + rsync_basepath=rsync_basepath, + ) + mock_capture_post.assert_called_once_with( + base_url=mock.ANY, + router_name="workflow_clem.router", + function_name="process_raw_tiffs", + token=context._token, + instrument_name=instrument_name, + session_id=session_id, + data={ + "series_name": mock.ANY, + "tiff_files": mock.ANY, + "series_metadata": str(dst_metadata), + }, + ) diff --git a/tests/client/contexts/test_fib.py b/tests/client/contexts/test_fib.py index 857133d83..756362073 100644 --- a/tests/client/contexts/test_fib.py +++ b/tests/client/contexts/test_fib.py @@ -630,20 +630,19 @@ def test_handle_autotem_metadata( "test_params", ( # Successful case - (True, True, True, True, True, True, True), # Using destination file path - (True, True, True, True, True, True, False), # Using client-side file path + (True, True, True, True, True, True), # Using destination file path + (True, True, True, True, True, False), # Using client-side file path # Early exits - (False, True, True, True, True, True, False), # No source - (True, False, True, True, True, True, False), # No destination - (True, True, False, True, True, True, False), # No site info - (True, True, True, False, True, True, False), # No project name - (True, True, True, True, False, True, False), # No stage position - (True, True, True, True, True, False, False), # No stage position values + (False, True, True, True, True, False), # No source + (True, False, True, True, True, False), # No site info + (True, True, False, True, True, False), # No project name + (True, True, True, False, True, False), # No stage position + (True, True, True, True, False, False), # No stage position values ), ) def test_make_drift_correction_gif( mocker: MockerFixture, - test_params: tuple[bool, bool, bool, bool, bool, bool, bool], + test_params: tuple[bool, bool, bool, bool, bool, bool], tmp_path: Path, visit_dir: Path, mock_machine_config: dict, @@ -652,7 +651,6 @@ def test_make_drift_correction_gif( # Unpack test params ( find_source, - find_dst, has_site_info, has_project_name, has_stage_position, @@ -681,12 +679,8 @@ def test_make_drift_correction_gif( mock_get_source.return_value = tmp_path if find_source else None mock_file_transferred_to = mocker.patch( - "murfey.client.contexts.fib._file_transferred_to" + "murfey.client.contexts.fib._file_transferred_to", side_effect=destination_files ) - if find_dst: - mock_file_transferred_to.side_effect = destination_files - else: - mock_file_transferred_to.return_value = None mock_capture_post = mocker.patch("murfey.client.contexts.fib.capture_post") @@ -781,11 +775,6 @@ def test_make_drift_correction_gif( if not find_source: mock_logger.warning.assert_called_with(f"No source found for file {file}") mock_capture_post.assert_not_called() - elif not find_dst: - mock_logger.warning.assert_called_with( - f"Could not find destination file path for {file.name!r}" - ) - mock_capture_post.assert_not_called() elif not has_site_info: mock_logger.debug.assert_called_with( f"No metadata found for site {lamella_num} yet"