From 4182dc7f82e80acfd197aaae14b1e53e850cbbb5 Mon Sep 17 00:00:00 2001 From: s-weigand Date: Fri, 6 Jan 2023 20:22:36 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=8C=20Improved=20error=20messages=20on?= =?UTF-8?q?=20file=20not=20found=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🧹 Removed code duplication in favor of single code path --- glotaran/project/project.py | 43 +++++++++++---------- glotaran/project/project_registry.py | 2 +- glotaran/project/project_result_registry.py | 26 +++++++++---- glotaran/project/test/test_project.py | 40 ++++++++++++++----- 4 files changed, 74 insertions(+), 37 deletions(-) diff --git a/glotaran/project/project.py b/glotaran/project/project.py index c49d25ebf..00fd0d33f 100644 --- a/glotaran/project/project.py +++ b/glotaran/project/project.py @@ -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, @@ -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, @@ -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, @@ -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``. @@ -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. diff --git a/glotaran/project/project_registry.py b/glotaran/project/project_registry.py index 38988adb8..f2d33b092 100644 --- a/glotaran/project/project_registry.py +++ b/glotaran/project/project_registry.py @@ -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: diff --git a/glotaran/project/project_result_registry.py b/glotaran/project/project_result_registry.py index fcefb7996..e216a333d 100644 --- a/glotaran/project/project_result_registry.py +++ b/glotaran/project/project_result_registry.py @@ -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``, @@ -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: @@ -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. diff --git a/glotaran/project/test/test_project.py b/glotaran/project/test/test_project.py index 9ccc2967e..808e98654 100644 --- a/glotaran/project/test/test_project.py +++ b/glotaran/project/test/test_project.py @@ -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) @@ -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):