Skip to content

Commit

Permalink
👌 Improved error messages on file not found error
Browse files Browse the repository at this point in the history
🧹 Removed code duplication in favor of single code path
  • Loading branch information
s-weigand committed Jan 7, 2023
1 parent 434c10b commit 4182dc7
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 37 deletions.
43 changes: 23 additions & 20 deletions glotaran/project/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,11 @@ def load_data(self, dataset_name: str) -> xr.Dataset | xr.DataArray:
"""
try:
return self._data_registry.load_item(dataset_name)
except ValueError as e:
raise ValueError(f"Dataset {dataset_name!r} does not exist.") from e
except ValueError as err:
raise ValueError(
f"Dataset {dataset_name!r} does not exist. "
f"Known Datasets are: {list(self._data_registry.items.keys())}"
) from err

def import_data(
self,
Expand Down Expand Up @@ -238,8 +241,11 @@ def load_model(self, name: str) -> Model:
"""
try:
return self._model_registry.load_item(name)
except ValueError as e:
raise ValueError(f"Model {name!r} does not exist.") from e
except ValueError as err:
raise ValueError(
f"Model {name!r} does not exist. "
f"Known Models are: {list(self._model_registry.items.keys())}"
) from err

def generate_model(
self,
Expand Down Expand Up @@ -327,8 +333,11 @@ def load_parameters(self, parameters_name: str) -> Parameters:
"""
try:
return self._parameter_registry.load_item(parameters_name)
except ValueError as e:
raise ValueError(f"Parameters '{parameters_name}' does not exist.") from e
except ValueError as err:
raise ValueError(
f"Parameters '{parameters_name}' does not exist. "
f"Known Parameters are: {list(self._parameter_registry.items.keys())}"
) from err

def generate_parameters(
self,
Expand Down Expand Up @@ -417,16 +426,11 @@ def get_result_path(self, result_name: str, *, latest: bool = False) -> Path:
------
ValueError
Raised if result does not exist.
"""
result_name = self._result_registry._latest_result_name_fallback(
result_name, latest=latest
)
path = self._result_registry.directory / result_name
if self._result_registry.is_item(path):
return path
raise ValueError(f"Result {result_name!r} does not exist.")
.. # noqa: DAR402
"""
return self._result_registry._latest_result_path_fallback(result_name, latest=latest)

def get_latest_result_path(self, result_name: str) -> Path:
"""Get the path to a result with name ``name``.
Expand Down Expand Up @@ -471,14 +475,13 @@ def load_result(self, result_name: str, *, latest: bool = False) -> Result:
------
ValueError
Raised if result does not exist.
.. # noqa: DAR402
"""
result_name = self._result_registry._latest_result_name_fallback(
result_name, latest=latest
return self._result_registry._loader(
self._result_registry._latest_result_path_fallback(result_name, latest=latest)
)
try:
return self._result_registry.load_item(result_name)
except ValueError as e:
raise ValueError(f"Result {result_name!r} does not exist.") from e

def load_latest_result(self, result_name: str) -> Result:
"""Load a result.
Expand Down
2 changes: 1 addition & 1 deletion glotaran/project/project_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def load_item(self, name: str) -> Any:
if name in self.items:
return self._loader(self.items[name])
raise ValueError(
f"No Item with name '{name}' exists. Known items are: {self.items.keys()}"
f"No Item with name '{name}' exists. Known items are: {list(self.items.keys())}"
)

def markdown(self, join_indentation: int = 0) -> MarkdownStr:
Expand Down
26 changes: 19 additions & 7 deletions glotaran/project/project_result_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def previous_result_paths(self, base_name: str) -> list[Path]:
"""
return sorted(self.directory.glob(f"{base_name}_run_*"))

def _latest_result_name_fallback(self, name: str, *, latest: bool = False) -> str:
def _latest_result_path_fallback(self, name: str, *, latest: bool = False) -> Path:
"""Fallback when a user forgets to specify the run to get a result.
If ``name`` contains the run number this will just return ``name``,
Expand All @@ -69,14 +69,20 @@ def _latest_result_name_fallback(self, name: str, *, latest: bool = False) -> st
Parameters
----------
name: str
Name of the result, which should contain the run specifyer.
Name of the result, which should contain the run specifier.
latest: bool
Flag to deactivate warning about using latest result. Defaults to False
Flag to deactivate warning about using latest result. Defaults to False.
Returns
-------
str
Name used to retrieve a result.
Path
Path to the result (latest result if ``name`` does not match the result pattern).
Raises
------
ValueError
Raised if result does not exist.
"""
if re.match(self.result_pattern, name) is None:
if latest is False:
Expand All @@ -89,8 +95,14 @@ def _latest_result_name_fallback(self, name: str, *, latest: bool = False) -> st
stacklevel=3,
)
previous_result_paths = self.previous_result_paths(name) or [Path(name)]
return previous_result_paths[-1].stem
return name
name = previous_result_paths[-1].stem
path = self._directory / name
if self.is_item(path):
return path

raise ValueError(
f"Result {name!r} does not exist. " f"Known Results are: {list(self.items.keys())}"
)

def create_result_run_name(self, base_name: str) -> str:
"""Create a result name for a model.
Expand Down
40 changes: 31 additions & 9 deletions glotaran/project/test/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ def test_generators_allow_overwrite(project_folder: Path, project_file: Path):
assert len(list(filter(lambda p: p.label.startswith("rates"), parameters.all()))) == 3


def test_missing_file_errors(tmp_path: Path):
def test_missing_file_errors(tmp_path: Path, project_folder: Path):
"""Error when accessing non existing files."""
with pytest.raises(FileNotFoundError) as exc_info:
Project.open(tmp_path, create_if_not_exist=False)
Expand All @@ -355,42 +355,64 @@ def test_missing_file_errors(tmp_path: Path):
== f"Project file {(tmp_path/'project.gta').as_posix()} does not exists."
)

project = Project.open(tmp_path)
project = Project.open(project_folder)

with pytest.raises(ValueError) as exc_info:
project.load_data("not-existing")

assert str(exc_info.value) == "Dataset 'not-existing' does not exist."
assert str(exc_info.value) == (
"Dataset 'not-existing' does not exist. "
"Known Datasets are: ['dataset_1', 'dataset_1-bad', 'test_data']"
)

with pytest.raises(ValueError) as exc_info:
project.load_model("not-existing")

assert str(exc_info.value) == "Model 'not-existing' does not exist."
assert str(exc_info.value) == (
"Model 'not-existing' does not exist. "
"Known Models are: ['test_model', 'test_model-bad']"
)

with pytest.raises(ValueError) as exc_info:
project.load_parameters("not-existing")

assert str(exc_info.value) == "Parameters 'not-existing' does not exist."
assert str(exc_info.value) == (
"Parameters 'not-existing' does not exist. "
"Known Parameters are: ['test_parameters', 'test_parameters-bad']"
)

with pytest.raises(ValueError) as exc_info:
project.load_result("not-existing_run_0000")

assert str(exc_info.value) == "Result 'not-existing_run_0000' does not exist."
expected_known_results = (
"Known Results are: "
"['sequential_run_0000', 'sequential_run_0001', 'test_run_0000', 'test_run_0001']"
)

assert str(exc_info.value) == (
f"Result 'not-existing_run_0000' does not exist. {expected_known_results}"
)

with pytest.raises(ValueError) as exc_info:
project.load_latest_result("not-existing")

assert str(exc_info.value) == "Result 'not-existing' does not exist."
assert str(exc_info.value) == (
f"Result 'not-existing' does not exist. {expected_known_results}"
)

with pytest.raises(ValueError) as exc_info:
project.get_result_path("not-existing_run_0000")

assert str(exc_info.value) == "Result 'not-existing_run_0000' does not exist."
assert str(exc_info.value) == (
f"Result 'not-existing_run_0000' does not exist. {expected_known_results}"
)

with pytest.raises(ValueError) as exc_info:
project.get_latest_result_path("not-existing")

assert str(exc_info.value) == "Result 'not-existing' does not exist."
assert str(exc_info.value) == (
f"Result 'not-existing' does not exist. {expected_known_results}"
)


def test_markdown_repr(project_folder: Path, project_file: Path):
Expand Down

0 comments on commit 4182dc7

Please sign in to comment.