diff --git a/docs/changes/newsfragments/127.bugfix b/docs/changes/newsfragments/127.bugfix new file mode 100644 index 000000000..2391242de --- /dev/null +++ b/docs/changes/newsfragments/127.bugfix @@ -0,0 +1 @@ +Fix a bug in which relative storage URIs will be computed relative to the CWD and not to the location of the YAML file by `Fede Raimondo`_. \ No newline at end of file diff --git a/junifer/api/parser.py b/junifer/api/parser.py index 8111f39f6..8e96b365e 100644 --- a/junifer/api/parser.py +++ b/junifer/api/parser.py @@ -71,4 +71,23 @@ def parse_yaml(filepath: Union[str, Path]) -> Dict: logger.info(f"Importing module: {t_module}") importlib.import_module(t_module) + # Compute path for the URI parameter in storage files that are relative + # This is a tricky thing that appeared in #127. The problem is that + # the path in the URI parameter is relative to YAML file, not to the + # current working directory. If we leave it as is in the contents + # dict, then it will be used later in the ``build`` function as is, + # which will be computed relative to the current working directory. + # The solution is to compute the absolute path and replace the + # relative path in the contents dict with the absolute path. + + # Check if the storage file is defined + if "storage" in contents: + if "uri" in contents["storage"]: + # Check if the storage file is relative + uri_path = Path(contents["storage"]["uri"]) + if not uri_path.is_absolute(): + # Compute the absolute path + contents["storage"]["uri"] = str( + (filepath.parent / uri_path).resolve() + ) return contents diff --git a/junifer/api/tests/test_parser.py b/junifer/api/tests/test_parser.py index 6585a1809..bf5bcb58d 100644 --- a/junifer/api/tests/test_parser.py +++ b/junifer/api/tests/test_parser.py @@ -77,6 +77,28 @@ def test_parse_yaml_failure_with_multi_module_autoload(tmp_path: Path) -> None: parse_yaml(fname) +def test_parse_yaml_with_wrong_path(tmp_path: Path) -> None: + """Test YAML parsing with wrong paths in with. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ + t_tmp_path = tmp_path / "test_relative_with" + # Write yaml that includes a relative path + yaml_path = t_tmp_path / "yamls" + yaml_path.mkdir(exist_ok=True, parents=True) + yaml_fname = yaml_path / "test_parse_yaml_wrong_path.yaml" + + yaml_fname.write_text("foo: bar\nwith:\n - missingt.py\n - scipy\n") + + # Check test file + with pytest.raises(ValueError, match="does not exist"): + parse_yaml(yaml_fname) + + def test_parse_yaml_relative_path(tmp_path: Path) -> None: """Test YAML parsing with relative paths in with. @@ -135,3 +157,58 @@ def test_parse_yaml_absolute_path(tmp_path: Path) -> None: # Check test file parse_yaml(yaml_fname) + + +def test_parse_storage_uri_relative(tmp_path: Path) -> None: + """Test YAML parsing with storage and relative URI. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ + fname = tmp_path / "test_parse_yaml_with_storage_uri.yaml" + fname.write_text("foo: bar\nwith: numpy\nstorage:\n uri: test.db\n") + + contents = parse_yaml(fname) + assert "foo" in contents + assert contents["foo"] == "bar" + assert "storage" in contents + assert "uri" in contents["storage"] + assert contents["storage"]["uri"] == str(tmp_path / "test.db") + + fname = tmp_path / "test_parse_yaml_with_storage_uri.yaml" + fname.write_text( + "foo: bar\nwith: numpy\nstorage:\n uri: ../another/test.db\n" + ) + + contents = parse_yaml(fname) + assert "foo" in contents + assert contents["foo"] == "bar" + assert "storage" in contents + assert "uri" in contents["storage"] + assert contents["storage"]["uri"] == str( + (tmp_path / "../another/test.db").resolve() + ) + + fname = tmp_path / "test_parse_yaml_with_storage_uri.yaml" + fname.write_text( + "foo: bar\nwith: numpy\nstorage:\n uri: /absolute/test.db\n" + ) + + contents = parse_yaml(fname) + assert "foo" in contents + assert contents["foo"] == "bar" + assert "storage" in contents + assert "uri" in contents["storage"] + assert contents["storage"]["uri"] == "/absolute/test.db" + + # Just to trick coverage + fname = tmp_path / "test_parse_yaml_with_storage_uri.yaml" + fname.write_text("foo: bar\nwith: numpy\nstorage:\n kind: SomeStorage\n") + + contents = parse_yaml(fname) + assert "foo" in contents + assert contents["foo"] == "bar" + assert "storage" in contents