🩹 Fix yaml result saving with relative paths

changelog.md

@@ -25,6 +25,7 @@
 - 🩹 Fix for normalization issue described in #1157 (multi-gaussian irfs and multiple time ranges (streak))
 - 🩹 Fix for crash described in #1183 when doing an optimization using more than 30 datasets (#1184)
 - 🩹 Fix pretty_format_numerical for negative values (#1192)
+- 🩹 Fix yaml result saving with relative paths (#1199)
 <!-- Fix within the 0.7.0 release cycle, therefore hidden:
 - 🩹 Fix the matrix provider alignment/reduction ('grouping') issues introduced in #1175 (#1190)
   -->

glotaran/builtin/io/yml/test/test_save_result.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from dataclasses import replace
+from functools import lru_cache
 from pathlib import Path
 from textwrap import dedent
 
@@ -9,26 +10,32 @@
 
 from glotaran import __version__
 from glotaran.io import load_result
+from glotaran.io import save_dataset
 from glotaran.io import save_result
 from glotaran.optimization.optimize import optimize
 from glotaran.project.result import Result
 from glotaran.testing.simulated_data.sequential_spectral_decay import SCHEME
+from glotaran.utils.io import chdir_context
 
 
-@pytest.fixture(scope="session")
-def dummy_result():
+@pytest.fixture
+def dummy_result(tmp_path: Path):
     """Dummy result for testing."""
-    print(SCHEME.data["dataset_1"])
-    scheme = replace(SCHEME, maximum_number_function_evaluations=1)
-    yield optimize(scheme, raise_exception=True)
 
+    @lru_cache
+    def create_result():
+        scheme = replace(SCHEME, maximum_number_function_evaluations=1)
+        (tmp_path / "data").mkdir(parents=True, exist_ok=True)
+        save_dataset(scheme.data["dataset_1"], tmp_path / "data/ds1.nc")
+        return optimize(scheme, raise_exception=True)
 
-def test_save_result_yml(
-    tmp_path: Path,
-    dummy_result: Result,
-):
+    yield create_result()
+
+
+@pytest.mark.parametrize("path_is_absolute", (True, False))
+def test_save_result_yml(tmp_path: Path, dummy_result: Result, path_is_absolute: bool):
     """Check all files exist."""
-    expected = dedent(
+    expected_result = dedent(
         f"""\
         number_of_function_evaluations: 1
         success: true
@@ -49,49 +56,86 @@ def test_save_result_yml(
           dataset_1: dataset_1.nc
         """
     )
+    expected_scheme = dedent(
+        """\
+        model: model.yml
+        parameters: initial_parameters.csv
+        data:
+          dataset_1: dataset_1.nc
+        clp_link_tolerance: 0.0
+        clp_link_method: nearest
+        maximum_number_function_evaluations: 1
+        add_svd: true
+        ftol: 1e-08
+        gtol: 1e-08
+        xtol: 1e-08
+        optimization_method: TrustRegionReflection
+        result_path: null
+        """
+    )
+    if path_is_absolute is True:
+        result_dir = tmp_path / "testresult"
+    else:
+        result_dir = Path("testresult")
+
+    assert (
+        dummy_result.scheme.data["dataset_1"].source_path == (tmp_path / "data/ds1.nc").as_posix()
+    )
 
-    result_dir = tmp_path / "testresult"
     result_path = result_dir / "result.yml"
-    save_result(result_path=result_path, result=dummy_result)
+    with chdir_context("." if path_is_absolute is True else tmp_path):
+        save_result(result_path=result_path, result=dummy_result)
 
-    assert dummy_result.source_path == result_path.as_posix()
+        assert dummy_result.source_path == result_path.as_posix()
 
-    assert (result_dir / "result.md").exists()
-    assert (result_dir / "scheme.yml").exists()
-    assert result_path.exists()
-    assert (result_dir / "initial_parameters.csv").exists()
-    assert (result_dir / "optimized_parameters.csv").exists()
-    assert (result_dir / "optimization_history.csv").exists()
-    assert (result_dir / "dataset_1.nc").exists()
+        assert (result_dir / "result.md").exists()
+        assert (result_dir / "scheme.yml").exists()
+        assert (result_dir / "scheme.yml").read_text() == expected_scheme
+        assert result_path.exists()
+        assert (result_dir / "initial_parameters.csv").exists()
+        assert (result_dir / "optimized_parameters.csv").exists()
+        assert (result_dir / "optimization_history.csv").exists()
+        assert (result_dir / "dataset_1.nc").exists()
+        # Original scheme object isn't changed
+        assert (
+            dummy_result.scheme.data["dataset_1"].source_path
+            == (tmp_path / "data/ds1.nc").as_posix()
+        )
 
-    # We can't check equality due to numerical fluctuations
-    got = result_path.read_text()
-    print(got)
-    assert expected in got
+        # We can't check equality due to numerical fluctuations
+        got = result_path.read_text()
+        print(got)
+        assert expected_result in got
 
 
-def test_save_result_yml_roundtrip(tmp_path: Path, dummy_result: Result):
+@pytest.mark.parametrize("path_is_absolute", (True, False))
+def test_save_result_yml_roundtrip(tmp_path: Path, dummy_result: Result, path_is_absolute: bool):
     """Save and reloaded Result should be the same."""
-    result_dir = tmp_path / "testresult"
+    if path_is_absolute is True:
+        result_dir = tmp_path / "testresult"
+    else:
+        result_dir = Path("testresult")
     result_path = result_dir / "result.yml"
-    save_result(result_path=result_path, result=dummy_result)
-    result_round_tripped = load_result(result_path)
 
-    assert dummy_result.source_path == result_path.as_posix()
-    assert result_round_tripped.source_path == result_path.as_posix()
+    with chdir_context("." if path_is_absolute is True else tmp_path):
+        save_result(result_path=result_path, result=dummy_result)
+        result_round_tripped = load_result(result_path)
 
-    assert_frame_equal(
-        dummy_result.initial_parameters.to_dataframe(),
-        result_round_tripped.initial_parameters.to_dataframe(),
-    )
-    assert_frame_equal(
-        dummy_result.optimized_parameters.to_dataframe(),
-        result_round_tripped.optimized_parameters.to_dataframe(),
-    )
-    assert_frame_equal(
-        dummy_result.parameter_history.to_dataframe(),
-        result_round_tripped.parameter_history.to_dataframe(),
-    )
-    assert_frame_equal(
-        dummy_result.optimization_history.data, result_round_tripped.optimization_history.data
-    )
+        assert dummy_result.source_path == result_path.as_posix()
+        assert result_round_tripped.source_path == result_path.as_posix()
+
+        assert_frame_equal(
+            dummy_result.initial_parameters.to_dataframe(),
+            result_round_tripped.initial_parameters.to_dataframe(),
+        )
+        assert_frame_equal(
+            dummy_result.optimized_parameters.to_dataframe(),
+            result_round_tripped.optimized_parameters.to_dataframe(),
+        )
+        assert_frame_equal(
+            dummy_result.parameter_history.to_dataframe(),
+            result_round_tripped.parameter_history.to_dataframe(),
+        )
+        assert_frame_equal(
+            dummy_result.optimization_history.data, result_round_tripped.optimization_history.data
+        )

glotaran/builtin/io/yml/test/test_save_scheme.py

@@ -1,7 +1,8 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING
+from pathlib import Path
 
+import pytest
 import xarray as xr
 
 from glotaran.io import load_scheme
@@ -13,10 +14,7 @@
 from glotaran.testing.simulated_data.sequential_spectral_decay import DATASET
 from glotaran.testing.simulated_data.sequential_spectral_decay import MODEL
 from glotaran.testing.simulated_data.sequential_spectral_decay import PARAMETERS
-
-if TYPE_CHECKING:
-    from pathlib import Path
-
+from glotaran.utils.io import chdir_context
 
 want = """\
 model: m.yml
@@ -35,7 +33,8 @@
 """
 
 
-def test_save_scheme(tmp_path: Path):
+@pytest.mark.parametrize("path_is_absolute", (True, False))
+def test_save_scheme(tmp_path: Path, path_is_absolute: bool):
     save_model(MODEL, tmp_path / "m.yml")
     save_parameters(PARAMETERS, tmp_path / "p.csv")
     save_dataset(DATASET, tmp_path / "d.nc")
@@ -44,12 +43,17 @@ def test_save_scheme(tmp_path: Path):
         PARAMETERS,
         {"dataset_1": DATASET},
     )
-    scheme_path = tmp_path / "testscheme.yml"
-    save_scheme(file_name=scheme_path, format_name="yml", scheme=scheme)
-
-    assert scheme_path.is_file()
-    assert scheme_path.read_text() == want
-    loaded = load_scheme(scheme_path)
-    print(loaded.model.validate(loaded.parameters))
-    assert loaded.model.valid(loaded.parameters)
-    assert isinstance(scheme.data["dataset_1"], xr.Dataset)
+    if path_is_absolute is True:
+        scheme_path = tmp_path / "testscheme.yml"
+    else:
+        scheme_path = Path("testscheme.yml")
+
+    with chdir_context("." if path_is_absolute is True else tmp_path):
+        save_scheme(file_name=scheme_path, format_name="yml", scheme=scheme)
+
+        assert scheme_path.is_file()
+        assert scheme_path.read_text() == want
+        loaded = load_scheme(scheme_path)
+        print(loaded.model.validate(loaded.parameters))
+        assert loaded.model.valid(loaded.parameters)
+        assert isinstance(scheme.data["dataset_1"], xr.Dataset)

glotaran/builtin/io/yml/yml.py

@@ -1,6 +1,7 @@
 """Module containing the YAML Data and Project IO plugins."""
 from __future__ import annotations
 
+from dataclasses import replace
 from pathlib import Path
 from typing import TYPE_CHECKING
 
@@ -214,8 +215,11 @@ def save_result(
         save_model(result.scheme.model, model_path, allow_overwrite=True)
         paths.append(model_path.as_posix())
 
+        # The source_path attribute of the datasets only gets changed for `result.data`
+        # Which why we overwrite the data attribute on a copy of `result.scheme`
+        scheme = replace(result.scheme, data=result.data)
         scheme_path = result_folder / "scheme.yml"
-        save_scheme(result.scheme, scheme_path, allow_overwrite=True)
+        save_scheme(scheme, scheme_path, allow_overwrite=True)
         paths.append(scheme_path.as_posix())
 
         result_dict = asdict(result, folder=result_folder)

glotaran/utils/io.py

@@ -9,6 +9,7 @@
 from collections.abc import Mapping
 from collections.abc import MutableMapping
 from collections.abc import Sequence
+from contextlib import contextmanager
 from pathlib import Path
 from typing import TYPE_CHECKING
 from typing import Any
@@ -19,6 +20,7 @@
 from glotaran.typing.types import DatasetMappable
 
 if TYPE_CHECKING:
+    from collections.abc import Generator
     from collections.abc import Iterator
 
     import pandas as pd
@@ -190,9 +192,42 @@ def load_datasets(dataset_mappable: DatasetMappable) -> DatasetMapping:
     return DatasetMapping.loader(dataset_mappable)
 
 
+@contextmanager
+def chdir_context(folder_path: StrOrPath) -> Generator[Path, None, None]:
+    """Context manager to change directory to ``folder_path``.
+
+    Parameters
+    ----------
+    folder_path: StrOrPath
+        Path to change to.
+
+    Yields
+    ------
+    Generator[Path, None, None]
+        Resolved path of ``folder_path``.
+
+    Raises
+    ------
+    ValueError
+        If ``folder_path`` is an existing file.
+    """
+    original_dir = Path(os.curdir).resolve()
+    folder_path = Path(folder_path)
+    if folder_path.is_file() is True:
+        raise ValueError("Value of 'folder_path' needs to be a folder but was an existing file.")
+    folder_path.mkdir(parents=True, exist_ok=True)
+    try:
+        os.chdir(folder_path)
+        yield folder_path.resolve()
+    finally:
+        os.chdir(original_dir)
+
+
 def relative_posix_path(source_path: StrOrPath, base_path: StrOrPath | None = None) -> str:
     """Ensure that ``source_path`` is a posix path, relative to ``base_path`` if defined.
 
+    For ``source_path`` to be converted to a relative path it either needs to a an absolute path or
+    ``base_path`` needs to be a parent directory of ``source_path``.
     On Windows if ``source_path`` and ``base_path`` are on different drives, it will return
     the absolute posix path to the file.
 
@@ -201,17 +236,20 @@ def relative_posix_path(source_path: StrOrPath, base_path: StrOrPath | None = No
     source_path : StrOrPath
         Path which should be converted to a relative posix path.
     base_path : StrOrPath, optional
-        Base path the resulting path string should be relative to., by default None
+        Base path the resulting path string should be relative to. Defaults to ``None``.
 
     Returns
     -------
     str
         ``source_path`` as posix path relative to ``base_path`` if defined.
     """
-    source_path = Path(source_path).as_posix()
-    if base_path is not None and os.path.isabs(source_path):
+    source_path = Path(source_path)
+    if base_path is not None and (
+        source_path.is_absolute() or Path(base_path).resolve() in source_path.resolve().parents
+    ):
         with contextlib.suppress(ValueError):
-            source_path = os.path.relpath(source_path, Path(base_path).as_posix())
+            source_path = os.path.relpath(source_path.as_posix(), Path(base_path).as_posix())
+
     return Path(source_path).as_posix()