🩹 Fix yaml result saving with relative paths

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

@@ -13,6 +13,7 @@
 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")
@@ -23,12 +24,10 @@ def dummy_result():
     yield optimize(scheme, raise_exception=True)
 
 
-def test_save_result_yml(
-    tmp_path: Path,
-    dummy_result: 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 +48,77 @@ 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")
 
-    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()
 
-    # 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/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()
 
 

glotaran/utils/test/test_io.py

@@ -2,6 +2,7 @@
 from __future__ import annotations
 
 import html
+import os
 import sys
 from pathlib import Path
 
@@ -19,6 +20,7 @@
 from glotaran.testing.simulated_data.sequential_spectral_decay import SCHEME
 from glotaran.testing.simulated_data.shared_decay import SPECTRAL_AXIS
 from glotaran.utils.io import DatasetMapping
+from glotaran.utils.io import chdir_context
 from glotaran.utils.io import create_clp_guide_dataset
 from glotaran.utils.io import load_datasets
 from glotaran.utils.io import relative_posix_path
@@ -196,11 +198,51 @@ def test_relative_posix_path(tmp_path: Path, rel_file_path: str):
 
     assert rel_result_path == rel_file_path
 
-    rel_result_no_coomon = relative_posix_path(
+    rel_result_no_common = relative_posix_path(
         (tmp_path / f"../{rel_file_path}").resolve().as_posix(), str(tmp_path)
     )
 
-    assert rel_result_no_coomon == f"../{rel_file_path}"
+    assert rel_result_no_common == f"../{rel_file_path}"
+
+    result_folder = tmp_path / "results"
+    with chdir_context(result_folder):
+        original_rel_path = relative_posix_path(rel_file_path, result_folder)
+        assert original_rel_path == rel_file_path
+
+        original_rel_path = relative_posix_path(rel_file_path, "not_a_parent")
+        assert original_rel_path == rel_file_path
+
+
+def test_chdir_context(tmp_path: Path):
+    """Original Path is restored even after exception is thrown."""
+    original_dir = Path(os.curdir).resolve()
+    with chdir_context(tmp_path) as curdir:
+        assert curdir == tmp_path.resolve()
+        assert tmp_path.resolve() == Path(os.curdir).resolve()
+        assert Path("test.txt").resolve() == (tmp_path / "test.txt").resolve()
+
+    assert Path(os.curdir).resolve() == original_dir
+
+    with pytest.raises(ValueError):
+        with chdir_context(tmp_path):
+            raise ValueError("Original path will be restored after I raise.")
+
+    assert Path(os.curdir).resolve() == original_dir
+
+
+def test_chdir_context_exception(tmp_path: Path):
+    """Raise error if ``folder_path`` is an existing file instead of a folder."""
+    file_path = tmp_path / "test.txt"
+    file_path.touch()
+
+    with pytest.raises(ValueError) as excinfo:
+        with chdir_context(file_path):
+            pass
+
+    assert (
+        str(excinfo.value)
+        == "Value of 'folder_path' needs to be a folder but was an existing file."
+    )
 
 
 @pytest.mark.skipif(not sys.platform.startswith("win32"), reason="Only needed for Windows")