diff --git a/src/murfey/client/analyser.py b/src/murfey/client/analyser.py index a16f2a7d..74f75926 100644 --- a/src/murfey/client/analyser.py +++ b/src/murfey/client/analyser.py @@ -398,7 +398,9 @@ def _xml_file(self, data_file: Path) -> Path: base_dir, mid_dir = find_longest_data_directory(data_file, data_directories) if not base_dir: return data_file.with_suffix(".xml") - return base_dir / self._environment.visit / mid_dir / file_name + # Add the visit directory to the file path and return it + # The file is moved from a location where the visit name is not part of its path + return base_dir / self._environment.visit / (mid_dir or "") / file_name def enqueue(self, rsyncer: RSyncerUpdate): if not self._stopping and rsyncer.outcome == TransferResult.SUCCESS: diff --git a/src/murfey/client/contexts/spa.py b/src/murfey/client/contexts/spa.py index 8aa2b7ab..6132f03d 100644 --- a/src/murfey/client/contexts/spa.py +++ b/src/murfey/client/contexts/spa.py @@ -3,7 +3,7 @@ import logging from itertools import count from pathlib import Path -from typing import Any, Dict, List, Optional, OrderedDict, Tuple +from typing import Any, Optional, OrderedDict import xmltodict @@ -50,10 +50,10 @@ def _file_transferred_to( def _grid_square_metadata_file( - f: Path, data_directories: List[Path], visit: str, grid_square: int + f: Path, data_directories: list[Path], visit: str, grid_square: int ) -> Path: base_dir, mid_dir = find_longest_data_directory(f, data_directories) - if not base_dir: + if not base_dir or not mid_dir: raise ValueError(f"Could not determine grid square metadata path for {f}") metadata_file = ( base_dir @@ -113,7 +113,7 @@ def __init__(self, acquisition_software: str, basepath: Path, token: str): super().__init__("SPA", acquisition_software, token) self._basepath = basepath self._processing_job_stash: dict = {} - self._foil_holes: Dict[int, List[int]] = {} + self._foil_holes: dict[int, list[int]] = {} def gather_metadata( self, metadata_file: Path, environment: MurfeyInstanceEnvironment | None = None @@ -287,7 +287,7 @@ def _position_analysis( and self._foil_holes.get(grid_square) is None ): self._foil_holes[grid_square] = [] - gs_pix_position: Tuple[ + gs_pix_position: tuple[ Optional[int], Optional[int], Optional[float], @@ -586,7 +586,7 @@ def _register_processing_job( self, tag: str, environment: MurfeyInstanceEnvironment, - parameters: Dict[str, Any] | None = None, + parameters: dict[str, Any] | None = None, ): return diff --git a/src/murfey/client/destinations.py b/src/murfey/client/destinations.py index c3ef6b6d..338d3820 100644 --- a/src/murfey/client/destinations.py +++ b/src/murfey/client/destinations.py @@ -15,7 +15,7 @@ def find_longest_data_directory( match_path: Path, data_directories: list[str] | list[Path] -): +) -> tuple[Path | None, Path | None]: """ Determine the longest path in the data_directories list which the match path is relative to @@ -23,9 +23,11 @@ def find_longest_data_directory( base_dir: Path | None = None mid_dir: Path | None = None for dd in data_directories: - dd_base = str(Path(dd).absolute()) - if str(match_path).startswith(str(dd)) and len(dd_base) > len(str(base_dir)): - base_dir = Path(dd_base) + dd_base = Path(dd).absolute() + if match_path.absolute().is_relative_to(dd_base) and len(dd_base.parts) > ( + len(base_dir.parts) if base_dir else 0 + ): + base_dir = dd_base mid_dir = match_path.absolute().relative_to(Path(base_dir)).parent return base_dir, mid_dir diff --git a/src/murfey/util/config.py b/src/murfey/util/config.py index 1407a8d7..6280bb7e 100644 --- a/src/murfey/util/config.py +++ b/src/murfey/util/config.py @@ -1,5 +1,6 @@ from __future__ import annotations +import copy import os import socket from functools import lru_cache @@ -183,7 +184,7 @@ def _recursive_update(base: dict[str, Any], new: dict[str, Any]): base[key].extend(value) # Otherwise, overwrite/add values as normal else: - base[key] = value + base[key] = copy.deepcopy(value) return base # Load the dict from the file diff --git a/tests/client/test_destinations.py b/tests/client/test_destinations.py index 54532d36..55375a86 100644 --- a/tests/client/test_destinations.py +++ b/tests/client/test_destinations.py @@ -2,8 +2,65 @@ from unittest import mock import pytest +from pytest_mock import MockerFixture + +from murfey.client.destinations import ( + determine_default_destination, + find_longest_data_directory, +) + + +@pytest.mark.parametrize( + "test_params", + ( + # File to match | Use Path? | Expected result + ( + "X:/cm12345-6/Supervisor/Images-Disc1/file", + True, + ("X:/", "cm12345-6/Supervisor/Images-Disc1"), + ), + ( + "X:/DATA/cm12345-6/Supervisor/Images-Disc1/file", + False, + ("X:/DATA", "cm12345-6/Supervisor/Images-Disc1"), + ), + ( + "X:/DoseFractions/cm12345-6/Supervisor/Images-Disc1/file", + True, + ("X:/DoseFractions", "cm12345-6/Supervisor/Images-Disc1"), + ), + ( + "X:/DoseFractions/DATA/cm12345-6/Supervisor/Images-Disc1/file", + False, + ("X:/DoseFractions/DATA", "cm12345-6/Supervisor/Images-Disc1"), + ), + ), +) +def test_find_longest_data_directory( + mocker: MockerFixture, test_params: tuple[str, bool, tuple[str, str]] +): + # Unpack test params + match_path, use_path, (expected_base_dir, expected_mid_dir) = test_params + + # Construct data directories using strings or Paths as needed + data_directories: list[str] | list[Path] = [ + "X:", + "X:/DATA", + "X:/DoseFractions", + "X:/DoseFractions/DATA", + ] + if use_path: + data_directories = [Path(dd) for dd in data_directories] + # Patch Pathlib's 'absolute()' function, since we are simulating Windows on Linux + mocker.patch("murfey.client.destinations.Path.absolute", lambda self: self) + + base_dir, mid_dir = find_longest_data_directory( + Path(match_path), + data_directories, + ) + assert base_dir == Path(expected_base_dir) + assert mid_dir == Path(expected_mid_dir) -from murfey.client.destinations import determine_default_destination source_list = [ ["X:/DoseFractions/cm12345-6/Supervisor", "Supervisor", True, "extra_name"], @@ -71,6 +128,56 @@ def test_determine_default_destinations_suggested_path(mock_post, mock_get, sour ) +@mock.patch("murfey.client.destinations.capture_get") +@mock.patch("murfey.client.destinations.capture_post") +def test_determine_default_destinations_short_path(mock_post, mock_get): + """Test for the case where the data directory is a drive""" + mock_environment = mock.Mock() + mock_environment.murfey_session = 2 + mock_environment.instrument_name = "m01" + mock_environment.destination_registry = {} + + mock_get().json.return_value = {"data_directories": ["X:/"]} + mock_post().json.return_value = {"suggested_path": "/base_path/2025/cm12345-6/raw"} + + destination = determine_default_destination( + visit="cm12345-6", + source=Path("X:/cm12345-6/Supervisor"), + destination="2025", + environment=mock_environment, + token="token", + touch=True, + extra_directory="", + use_suggested_path=True, + ) + mock_get.assert_any_call( + base_url=str(mock_environment.url.geturl()), + router_name="session_control.router", + function_name="machine_info_by_instrument", + token="token", + instrument_name="m01", + ) + mock_post.assert_any_call( + base_url=str(mock_environment.url.geturl()), + router_name="file_io_instrument.router", + function_name="suggest_path", + token="token", + visit_name="cm12345-6", + session_id=2, + data={ + "base_path": "2025/cm12345-6/raw", + "touch": True, + "extra_directory": "", + }, + ) + + assert destination == "/base_path/2025/cm12345-6/raw/" + assert ( + mock_environment.destination_registry.get("Supervisor") + == "/base_path/2025/cm12345-6/raw" + ) + + @mock.patch("murfey.client.destinations.capture_get") @pytest.mark.parametrize("sources", source_list) def test_determine_default_destinations_skip_suggested(mock_get, sources): diff --git a/tests/util/test_config.py b/tests/util/test_config.py index 82068a8c..5063f0c3 100644 --- a/tests/util/test_config.py +++ b/tests/util/test_config.py @@ -1,3 +1,4 @@ +import copy from pathlib import Path from typing import Any @@ -132,11 +133,23 @@ def mock_instrument_config(): } +@pytest.fixture +def mock_second_instrument_config(mock_instrument_config: dict[str, Any]): + """Test the case of recursive dictionaries which are present + in both the general and instrument configs""" + second_instrument_config = copy.deepcopy(mock_instrument_config) + second_instrument_config.update( + {"pkg_1": {"file_path": "/path/to/pkg_1/file_2.txt"}} + ) + return second_instrument_config + + @pytest.fixture def mock_hierarchical_machine_config_yaml( mock_general_config: dict[str, Any], mock_tem_shared_config: dict[str, Any], mock_instrument_config: dict[str, Any], + mock_second_instrument_config: dict[str, Any], tmp_path: Path, ): # Create machine config (with all currently supported keys) for the instrument @@ -144,7 +157,7 @@ def mock_hierarchical_machine_config_yaml( "general": mock_general_config, "tem": mock_tem_shared_config, "m01": mock_instrument_config, - "m02": mock_instrument_config, + "m02": mock_second_instrument_config, } config_file = tmp_path / "config" / "murfey-machine-config-hierarchical.yaml" config_file.parent.mkdir(parents=True, exist_ok=True) @@ -210,6 +223,7 @@ def test_get_machine_config( mock_general_config: dict[str, Any], mock_tem_shared_config: dict[str, Any], mock_instrument_config: dict[str, Any], + mock_second_instrument_config: dict[str, Any], mock_hierarchical_machine_config_yaml: Path, mock_standard_machine_config_yaml: Path, test_params: tuple[str, list[str]], @@ -353,14 +367,26 @@ def test_get_machine_config( == mock_instrument_config["node_creator_queue"] ) # Extra keys - assert config[i].pkg_1 == { - "file_path": "/path/to/pkg_1/file.txt", - "command": [ - "/path/to/executable", - "--some_arg", - "-a", - "./path/to/file", - ], - "step_size": 100, - } + if config_to_test == "standard" or i == "m01": + assert config[i].pkg_1 == { + "file_path": "/path/to/pkg_1/file.txt", + "command": [ + "/path/to/executable", + "--some_arg", + "-a", + "./path/to/file", + ], + "step_size": 100, + } + elif i == "m02": + assert config[i].pkg_1 == { + "file_path": "/path/to/pkg_1/file_2.txt", + "command": [ + "/path/to/executable", + "--some_arg", + "-a", + "./path/to/file", + ], + "step_size": 100, + } assert config[i].pkg_2 == mock_general_config["pkg_2"]