From 1e523033fe996ecec0542fbd4b9daa46d6b3ebf4 Mon Sep 17 00:00:00 2001 From: dorme Date: Tue, 31 Jan 2023 11:14:35 +0000 Subject: [PATCH 01/11] Removed log_and_raise, document logging practice for exceptions --- virtual_rainforest/core/logger.py | 63 +++++++++++++++++-------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/virtual_rainforest/core/logger.py b/virtual_rainforest/core/logger.py index d049388df..8ecda56ae 100644 --- a/virtual_rainforest/core/logger.py +++ b/virtual_rainforest/core/logger.py @@ -40,17 +40,44 @@ so that ``DEBUG`` messages are suppressed, except when we are actively trying to debug the model. -The logging module also defines a function that we should generally make use of to kill -the simulation when something goes wrong. This function -:func:`~virtual_rainforest.core.logger.log_and_raise` raises an exception (of the -developers choice) and adds a developer specified ``CRITICAL`` message to the log. This -function is useful as it ensures that ``CRITICAL`` logging events are accompanied by a -simulation ending exception. As such, this is probably the only means by which you -should log a ``CRITICAL`` message. +Logging and exceptions +---------------------- + +When an exception is allowed to halt the code, it is important for the reason to be +written to the log, as well as producing any traceback to the console. So, exception +handling should always include a LOGGER call, using one of the following patterns. + +#. A test in the code indicates that we should raise an exception: + + .. code-block:: python + + if thing_has_gone_wrong: + excep = ValueError("It went wrong!") + LOGGER.critical(excep) + raise excep + +#. A ``try`` block results in an exception: + + .. code-block:: python + + try: + doing_something_that_raises() + except ValueError as excep: + LOGGER.critical(excep) + raise + +#. A ``try`` block results in an exception and we want to change the exception type: + + .. code-block:: python + + try: + doing_something_that_raises() + except ValueError as excep: + LOGGER.critical(excep) + raise ValueError("Bad input") from excep """ # noqa: D205 import logging -from typing import Optional, Type logging.basicConfig( level=logging.INFO, @@ -58,23 +85,3 @@ ) LOGGER = logging.getLogger("virtual_rainforest") - - -def log_and_raise( - msg: str, exception: Type[BaseException], extra: Optional[dict] = None -) -> None: - """Emit a critical error message and raise an Exception. - - This convenience function adds a critical level message to the logger and - then raises an exception with the same message. This is intended only for - use in loading resources: the package cannot run properly with misconfigured - resources but errors with the data checking should log and carry on. - - Args: - msg: A message to add to the log - exception: An exception type to be raised - extra: A dictionary of extra information to be passed to the logger - """ - - LOGGER.critical(msg, extra=extra) - raise exception(msg) From 7f4994e1e3ec88e92b95a3a95783e536e188228a Mon Sep 17 00:00:00 2001 From: dorme Date: Tue, 31 Jan 2023 11:26:59 +0000 Subject: [PATCH 02/11] Updated logger notes for mypy chicanery, fixed entry_points --- virtual_rainforest/core/logger.py | 6 +++--- virtual_rainforest/entry_points.py | 20 +++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/virtual_rainforest/core/logger.py b/virtual_rainforest/core/logger.py index 8ecda56ae..5b902854a 100644 --- a/virtual_rainforest/core/logger.py +++ b/virtual_rainforest/core/logger.py @@ -52,9 +52,9 @@ .. code-block:: python if thing_has_gone_wrong: - excep = ValueError("It went wrong!") - LOGGER.critical(excep) - raise excep + to_raise = ValueError("It went wrong!") + LOGGER.critical(to_raise) + raise to_raise #. A ``try`` block results in an exception: diff --git a/virtual_rainforest/entry_points.py b/virtual_rainforest/entry_points.py index 33f580a9c..9d7d31b23 100644 --- a/virtual_rainforest/entry_points.py +++ b/virtual_rainforest/entry_points.py @@ -11,7 +11,7 @@ import virtual_rainforest as vr from virtual_rainforest.core.config import ConfigurationError -from virtual_rainforest.core.logger import log_and_raise +from virtual_rainforest.core.logger import LOGGER from virtual_rainforest.main import vr_run @@ -57,19 +57,21 @@ def _vr_run_cli() -> None: try: args = parser.parse_args() - except SystemExit as e: - if e.code != 0: + except SystemExit as excep: + if excep.code != 0: # Catch unexpected args and similar problems - log_and_raise("For more information please run vr_run --help", SystemExit) + exit = SystemExit("For more information please run vr_run --help") + LOGGER.critical(exit) + raise exit else: - raise SystemExit + raise excep if not args.cfg_paths: - log_and_raise( - "No configuration paths must be provided! For more information please run " - "vr_run --help", - ConfigurationError, + to_raise = ConfigurationError( + "Configuration paths must be provided! See vr_run --help" ) + LOGGER.critical(to_raise) + raise to_raise else: # Run the virtual rainforest run function vr_run(args.cfg_paths, Path(args.merge_file_path)) From 4322091a0ed89ecfcbd5a8458e794ed7bcd410ea Mon Sep 17 00:00:00 2001 From: dorme Date: Tue, 31 Jan 2023 12:00:52 +0000 Subject: [PATCH 03/11] Removed l&r from core.config --- virtual_rainforest/core/config.py | 128 ++++++++++++++++++------------ 1 file changed, 77 insertions(+), 51 deletions(-) diff --git a/virtual_rainforest/core/config.py b/virtual_rainforest/core/config.py index ab050095b..3463af709 100644 --- a/virtual_rainforest/core/config.py +++ b/virtual_rainforest/core/config.py @@ -47,13 +47,13 @@ def schema() -> dict: from collections import ChainMap from copy import deepcopy from pathlib import Path -from typing import Any, Callable, Iterator, Optional, Union +from typing import Any, Callable, Iterator, Optional, Union, Generator import dpath.util # type: ignore import tomli_w from jsonschema import Draft202012Validator, FormatChecker, exceptions, validators -from virtual_rainforest.core.logger import LOGGER, log_and_raise +from virtual_rainforest.core.logger import LOGGER if sys.version_info[:2] >= (3, 11): import tomllib @@ -70,9 +70,11 @@ def schema() -> dict: class ConfigurationError(Exception): """Custom exception class for configuration failures.""" + pass + def log_all_validation_errors( - errors: list[exceptions.ValidationError], complete: bool + errors: Generator[Any, None, None], complete: bool ) -> None: """Logs all validation errors and raises an exception. @@ -94,10 +96,11 @@ def log_all_validation_errors( tag += f"[{k}]" LOGGER.error("%s: %s" % (tag, error.message)) - log_and_raise( - f"Validation of {conf} configuration files failed see above errors", - ConfigurationError, + to_raise = ConfigurationError( + f"Validation of {conf} configuration files failed see above errors" ) + LOGGER.critical(to_raise) + raise to_raise def validate_and_add_defaults( @@ -152,31 +155,38 @@ def register_schema(module_name: str) -> Callable: """ def wrap(func: Callable) -> Callable: + + # Type the exception raising with a general base class + to_raise: Exception + if module_name in SCHEMA_REGISTRY: - log_and_raise( + to_raise = ValueError( f"The module schema {module_name} is used multiple times, this " f"shouldn't be the case!", - ValueError, ) + LOGGER.critical(to_raise) + raise to_raise else: # Check that this is a valid schema try: Draft202012Validator.check_schema(func()) except exceptions.SchemaError: - log_and_raise( - f"Module schema {module_name} not valid JSON!", - OSError, - ) + to_raise = OSError(f"Module schema {module_name} not valid JSON!") + LOGGER.critical(to_raise) + raise to_raise + # Check that relevant keys are included try: func()["properties"][module_name] func()["required"] except KeyError as err: - log_and_raise( + to_raise = KeyError( f"Schema for {module_name} module incorrectly structured, {err} key" - f" missing!", - KeyError, + f" missing!" ) + LOGGER.critical(to_raise) + raise to_raise + # If it is valid then add it to the registry SCHEMA_REGISTRY[module_name] = func() @@ -235,25 +245,29 @@ def check_outfile(merge_file_path: Path) -> None: # Throw critical error if the output folder doesn't exist if not Path(parent_fold).exists(): - log_and_raise( - f"The user specified output directory ({parent_fold}) doesn't exist!", - ConfigurationError, + to_raise = ConfigurationError( + f"The user specified output directory ({parent_fold}) doesn't exist!" ) + LOGGER.critical(to_raise) + raise to_raise + elif not Path(parent_fold).is_dir(): - log_and_raise( - f"The user specified output folder ({parent_fold}) isn't a directory!", - ConfigurationError, + to_raise = ConfigurationError( + f"The user specified output folder ({parent_fold}) isn't a directory!" ) + LOGGER.critical(to_raise) + raise to_raise # Throw critical error if combined output file already exists for file in Path(parent_fold).iterdir(): if file.name == f"{out_file_name}": - log_and_raise( + to_raise = ConfigurationError( f"A config file in the user specified output folder ({parent_fold}) " f"already makes use of the specified output file name ({out_file_name})" - f", this file should either be renamed or deleted!", - ConfigurationError, + f", this file should either be renamed or deleted!" ) + LOGGER.critical(to_raise) + raise to_raise return None @@ -292,24 +306,29 @@ def collect_files(cfg_paths: list[str]) -> list[Path]: # Check for items that are not found if len(not_found) != 0: - log_and_raise( - f"The following (user provided) config paths do not exist:\n{not_found}", - ConfigurationError, + to_raise = ConfigurationError( + f"The following (user provided) config paths do not exist:\n{not_found}" ) + LOGGER.critical(to_raise) + raise to_raise + # And for empty folders - elif len(empty_fold) != 0: - log_and_raise( + if len(empty_fold) != 0: + to_raise = ConfigurationError( f"The following (user provided) config folders do not contain any toml " - f"files:\n{empty_fold}", - ConfigurationError, + f"files:\n{empty_fold}" ) + LOGGER.critical(to_raise) + raise to_raise + # Finally check that no files are pointed to twice - elif len(files) != len(set(files)): - log_and_raise( + if len(files) != len(set(files)): + to_raise = ConfigurationError( f"A total of {len(files) - len(set(files))} config files are specified more" - f" than once (possibly indirectly)", - ConfigurationError, + f" than once (possibly indirectly)" ) + LOGGER.critical(to_raise) + raise to_raise return files @@ -344,11 +363,12 @@ def load_in_config_files(files: list[Path]) -> dict[str, Any]: file_data[file] = toml_dict except tomllib.TOMLDecodeError as err: - log_and_raise( + to_raise = ConfigurationError( f"Configuration file {file} is incorrectly formatted. Failed with " - f"the following message:\n{err}", - ConfigurationError, + f"the following message:\n{err}" ) + LOGGER.critical(to_raise) + raise to_raise # Check if any tags are repeated across files if len(conflicts) != 0: @@ -357,7 +377,9 @@ def load_in_config_files(files: list[Path]) -> dict[str, Any]: for conf in conflicts: msg += f"{conf[0]} defined in both {conf[1]} and {conf[2]}\n" msg = msg[:-1] - log_and_raise(msg, ConfigurationError) + to_raise = ConfigurationError(msg) + LOGGER.critical(to_raise) + raise to_raise # Merge everything into a single dictionary config_dict = dict(ChainMap(*file_data.values())) @@ -384,10 +406,11 @@ def add_core_defaults(config_dict: dict[str, Any]) -> None: if "core" in SCHEMA_REGISTRY: core_schema = SCHEMA_REGISTRY["core"] else: - log_and_raise( - "Expected a schema for core module configuration, it was not provided!", - ConfigurationError, + to_raise = ConfigurationError( + "Expected a schema for core module configuration, it was not provided!" ) + LOGGER.critical(to_raise) + raise to_raise try: ValidatorWithDefaults(core_schema, format_checker=FormatChecker()).validate( @@ -417,22 +440,24 @@ def find_schema(config_dict: dict[str, Any]) -> list[str]: try: modules = config_dict["core"]["modules"] except KeyError: - log_and_raise( + to_raise = ConfigurationError( "Core configuration does not specify which other modules should be " - "configured!", - ConfigurationError, + "configured!" ) + LOGGER.critical(to_raise) + raise to_raise # Add core to list of modules if its not already included if "core" not in modules: modules.append("core") if len(modules) != len(set(modules)): - log_and_raise( + to_raise = ConfigurationError( f"The list of modules to configure given in the core configuration file " - f"repeats {len(modules) - len(set(modules))} names!", - ConfigurationError, + f"repeats {len(modules) - len(set(modules))} names!" ) + LOGGER.critical(to_raise) + raise to_raise return modules @@ -463,11 +488,12 @@ def construct_combined_schema(modules: list[str]) -> dict[str, Any]: # Add module name to list of required modules comb_schema["required"].append(module) else: - log_and_raise( + to_raise = ConfigurationError( f"Expected a schema for {module} module configuration, it was not " - f"provided!", - ConfigurationError, + f"provided!" ) + LOGGER.critical(to_raise) + raise to_raise p_paths = [] # Recursively search for all instances of properties in the schema From 69ca9c0af8711d31b0bba2e61c8262252ff9c820 Mon Sep 17 00:00:00 2001 From: dorme Date: Tue, 31 Jan 2023 12:38:46 +0000 Subject: [PATCH 04/11] Removed l&r from soil.model --- virtual_rainforest/soil/model.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/virtual_rainforest/soil/model.py b/virtual_rainforest/soil/model.py index 35e2a67ba..b83ff66fd 100644 --- a/virtual_rainforest/soil/model.py +++ b/virtual_rainforest/soil/model.py @@ -24,7 +24,7 @@ class as a child of the :class:`~virtual_rainforest.core.model.BaseModel` class. import pint from numpy import datetime64, timedelta64 -from virtual_rainforest.core.logger import LOGGER, log_and_raise +from virtual_rainforest.core.logger import LOGGER from virtual_rainforest.core.model import BaseModel, InitialisationError @@ -55,14 +55,18 @@ def __init__( ): if no_layers < 1: - log_and_raise( - "There has to be at least one soil layer in the soil model!", - InitialisationError, + to_raise = InitialisationError( + "There has to be at least one soil layer in the soil model!" ) - elif no_layers != int(no_layers): - log_and_raise( - "The number of soil layers must be an integer!", InitialisationError + LOGGER.critical(to_raise) + raise to_raise + + if no_layers != int(no_layers): + to_raise = InitialisationError( + "The number of soil layers must be an integer!" ) + LOGGER.critical(to_raise) + raise to_raise super().__init__(update_interval, start_time, **kwargs) self.no_layers = int(no_layers) From a4443ca9349530f58328142a5d4e2c97c5c9422f Mon Sep 17 00:00:00 2001 From: dorme Date: Tue, 31 Jan 2023 12:44:51 +0000 Subject: [PATCH 05/11] Removed l&r from main --- virtual_rainforest/main.py | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/virtual_rainforest/main.py b/virtual_rainforest/main.py index 7702ac145..74d0b449e 100644 --- a/virtual_rainforest/main.py +++ b/virtual_rainforest/main.py @@ -12,7 +12,7 @@ from numpy import datetime64, timedelta64 from virtual_rainforest.core.config import validate_config -from virtual_rainforest.core.logger import LOGGER, log_and_raise +from virtual_rainforest.core.logger import LOGGER from virtual_rainforest.core.model import MODEL_REGISTRY, BaseModel, InitialisationError @@ -40,11 +40,12 @@ def select_models(model_list: list[str]) -> list[Type[BaseModel]]: # Make list of missing models, and return an error if necessary miss_model = [model for model in model_list_ if model not in MODEL_REGISTRY.keys()] if miss_model: - log_and_raise( + to_raise = InitialisationError( f"The following models cannot be configured as they are not found in the " - f"registry: {miss_model}", - InitialisationError, + f"registry: {miss_model}" ) + LOGGER.critical(to_raise) + raise to_raise # Then extract each model from the registry modules = [MODEL_REGISTRY[model] for model in model_list_] @@ -76,11 +77,12 @@ def configure_models( # If any models fail to configure inform the user about it if failed_models: - log_and_raise( + to_raise = InitialisationError( f"Could not configure all the desired models, ending the simulation. The " - f"following models failed: {failed_models}.", - InitialisationError, + f"following models failed: {failed_models}." ) + LOGGER.critical(to_raise) + raise to_raise return models_cfd @@ -112,11 +114,12 @@ def extract_timing_details( try: raw_length = pint.Quantity(config["core"]["timing"]["run_length"]).to("minutes") except (pint.errors.DimensionalityError, pint.errors.UndefinedUnitError): - log_and_raise( + to_raise = InitialisationError( "Units for core.timing.run_length are not valid time units: %s" - % config["core"]["timing"]["run_length"], - InitialisationError, + % config["core"]["timing"]["run_length"] ) + LOGGER.critical(to_raise) + raise to_raise else: # Round raw time interval to nearest minute run_length = timedelta64(int(round(raw_length.magnitude)), "m") @@ -127,21 +130,24 @@ def extract_timing_details( "minutes" ) except (pint.errors.DimensionalityError, pint.errors.UndefinedUnitError): - log_and_raise( + to_raise = InitialisationError( "Units for core.timing.update_interval are not valid time units: %s" - % config["core"]["timing"]["update_interval"], - InitialisationError, + % config["core"]["timing"]["update_interval"] ) + LOGGER.critical(to_raise) + raise to_raise + else: # Round raw time interval to nearest minute update_interval = timedelta64(int(round(raw_interval.magnitude)), "m") if run_length < update_interval: - log_and_raise( + to_raise = InitialisationError( f"Models will never update as the update interval ({update_interval}) is " - f"larger than the run length ({run_length})", - InitialisationError, + f"larger than the run length ({run_length})" ) + LOGGER.critical(to_raise) + raise to_raise # Calculate when the simulation should stop based on principle that it should run at # least as long as the user requests rather than shorter From d89fa6f05e947a897679df0406e128d95f2cfddc Mon Sep 17 00:00:00 2001 From: dorme Date: Tue, 31 Jan 2023 12:55:18 +0000 Subject: [PATCH 06/11] Removed l&r from core.axes and core.data --- virtual_rainforest/core/axes.py | 19 ++++++++++++------- virtual_rainforest/core/data.py | 18 ++++++++++++++---- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/virtual_rainforest/core/axes.py b/virtual_rainforest/core/axes.py index c02223a51..3ddb28e30 100644 --- a/virtual_rainforest/core/axes.py +++ b/virtual_rainforest/core/axes.py @@ -54,7 +54,7 @@ from xarray import DataArray from virtual_rainforest.core.grid import Grid -from virtual_rainforest.core.logger import LOGGER, log_and_raise +from virtual_rainforest.core.logger import LOGGER class AxisValidator(ABC): @@ -210,6 +210,7 @@ def validate_dataarray( """ validation_dict: dict[str, Optional[str]] = {} + to_raise: Exception # Get the validators applying to each axis for axis in AXIS_VALIDATORS: @@ -229,16 +230,19 @@ def validate_dataarray( validator_found = [v for v in validators if v().can_validate(value, grid)] if len(validator_found) == 0: - log_and_raise( + to_raise = ValueError( f"DataArray uses '{axis}' axis dimension names but does " - f"not match a validator: {','.join(matching_dims)}", - ValueError, + f"not match a validator: {','.join(matching_dims)}" ) + LOGGER.critical(to_raise) + raise to_raise if len(validator_found) > 1: - log_and_raise( - f"Validators on '{axis}' axis not mutually exclusive", RuntimeError + to_raise = RuntimeError( + f"Validators on '{axis}' axis not mutually exclusive" ) + LOGGER.critical(to_raise) + raise to_raise # Get the appropriate Validator class and then use it to update the data # array @@ -246,7 +250,8 @@ def validate_dataarray( try: value = this_validator().run_validation(value, grid, **kwargs) except Exception as excep: - log_and_raise(str(excep), excep.__class__) + LOGGER.critical(excep) + raise validation_dict[axis] = this_validator.__name__ diff --git a/virtual_rainforest/core/data.py b/virtual_rainforest/core/data.py index 7a36f0aa9..f41ebaccc 100644 --- a/virtual_rainforest/core/data.py +++ b/virtual_rainforest/core/data.py @@ -128,7 +128,7 @@ from virtual_rainforest.core.axes import AXIS_VALIDATORS, validate_dataarray from virtual_rainforest.core.config import ConfigurationError from virtual_rainforest.core.grid import Grid -from virtual_rainforest.core.logger import LOGGER, log_and_raise +from virtual_rainforest.core.logger import LOGGER from virtual_rainforest.core.readers import load_to_dataarray @@ -142,13 +142,18 @@ class Data: Args: grid: The Grid instance that will be used for simulation. + + Raises: + TypeError: when grid is not a Grid object """ def __init__(self, grid: Grid) -> None: # Set up the instance properties if not isinstance(grid, Grid): - log_and_raise("Data must be initialised with a Grid object", TypeError) + to_raise = TypeError("Data must be initialised with a Grid object") + LOGGER.critical(to_raise) + raise to_raise self.grid: Grid = grid """The configured Grid to be used in a simulation.""" @@ -188,12 +193,17 @@ def __setitem__(self, key: str, value: DataArray) -> None: Args: key: The name to store the data under value: The DataArray to be stored + + Raises: + TypeError: when the value is not a DataArray. """ if not isinstance(value, DataArray): - log_and_raise( - "Only DataArray objects can be added to Data instances", TypeError + to_raise = TypeError( + "Only DataArray objects can be added to Data instances" ) + LOGGER.critical(to_raise) + raise to_raise if key not in self.data.data_vars: LOGGER.info(f"Adding data array for '{key}'") From d0b3846b7d3d89e4eeb390725140a84e6890ba8e Mon Sep 17 00:00:00 2001 From: dorme Date: Tue, 31 Jan 2023 14:24:15 +0000 Subject: [PATCH 07/11] Removed l&r from core.axes and core.readers --- virtual_rainforest/core/readers.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/virtual_rainforest/core/readers.py b/virtual_rainforest/core/readers.py index e48a0618e..addd03b3e 100644 --- a/virtual_rainforest/core/readers.py +++ b/virtual_rainforest/core/readers.py @@ -39,7 +39,7 @@ def new_function_to_load_tif_data(...): from xarray import DataArray, load_dataset -from virtual_rainforest.core.logger import LOGGER, log_and_raise +from virtual_rainforest.core.logger import LOGGER FILE_FORMAT_REGISTRY: dict[str, Callable] = {} """A registry for different file format loaders @@ -106,23 +106,35 @@ def load_netcdf(file: Path, var_name: str) -> DataArray: Args: file: A Path for a NetCDF file containing the variable to load. var_name: A string providing the name of the variable in the file. + + Raises: + FileNotFoundError: with bad file path names. + ValueError: if the file data is not readable. + KeyError: if the named variable is not present in the data. """ # Note that this deliberately doesn't contain any INFO logging messages to maintain # a simple logging sequence without unnecessary logger noise about the specific # format unless there is an exception. + to_raise: Exception # Try and load the provided file try: dataset = load_dataset(file) except FileNotFoundError: - log_and_raise(f"Data file not found: {file}", FileNotFoundError) + to_raise = FileNotFoundError(f"Data file not found: {file}") + LOGGER.critical(to_raise) + raise to_raise except ValueError as err: - log_and_raise(f"Could not load data from {file}: {err}.", ValueError) + to_raise = ValueError(f"Could not load data from {file}: {err}.") + LOGGER.critical(to_raise) + raise to_raise # Check if file var is in the dataset if var_name not in dataset: - log_and_raise(f"Variable {var_name} not found in {file}", KeyError) + to_raise = KeyError(f"Variable {var_name} not found in {file}") + LOGGER.critical(to_raise) + raise to_raise return dataset[var_name] @@ -142,6 +154,9 @@ def load_to_dataarray( Args: file: A Path for the file containing the variable to load. var_name: A string providing the name of the variable in the file. + + Raises: + ValueError: if there is no loader provided for the file format. """ # Detect file type @@ -149,7 +164,9 @@ def load_to_dataarray( # Can the data mapper handle this grid and file type combination? if file_type not in FILE_FORMAT_REGISTRY: - log_and_raise(f"No file format loader provided for {file_type}", ValueError) + to_raise = ValueError(f"No file format loader provided for {file_type}") + LOGGER.critical(to_raise) + raise to_raise # If so, load the data LOGGER.info("Loading variable '%s' from file: %s", var_name, file) From 66145365b387a70aa14bb004304cb43df9a21f4d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 31 Jan 2023 14:39:42 +0000 Subject: [PATCH 08/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- virtual_rainforest/core/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virtual_rainforest/core/config.py b/virtual_rainforest/core/config.py index 3463af709..122a25b4b 100644 --- a/virtual_rainforest/core/config.py +++ b/virtual_rainforest/core/config.py @@ -47,7 +47,7 @@ def schema() -> dict: from collections import ChainMap from copy import deepcopy from pathlib import Path -from typing import Any, Callable, Iterator, Optional, Union, Generator +from typing import Any, Callable, Generator, Iterator, Optional, Union import dpath.util # type: ignore import tomli_w From d7d16915499890bda98ca0c04338ca34ba9d8482 Mon Sep 17 00:00:00 2001 From: dorme Date: Tue, 31 Jan 2023 14:40:58 +0000 Subject: [PATCH 09/11] Missed an isort error --- virtual_rainforest/core/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virtual_rainforest/core/config.py b/virtual_rainforest/core/config.py index 3463af709..122a25b4b 100644 --- a/virtual_rainforest/core/config.py +++ b/virtual_rainforest/core/config.py @@ -47,7 +47,7 @@ def schema() -> dict: from collections import ChainMap from copy import deepcopy from pathlib import Path -from typing import Any, Callable, Iterator, Optional, Union, Generator +from typing import Any, Callable, Generator, Iterator, Optional, Union import dpath.util # type: ignore import tomli_w From 4218771d1ed10085fba81affd98d36e959e84c95 Mon Sep 17 00:00:00 2001 From: Jacob Cook Date: Wed, 1 Feb 2023 10:06:26 +0000 Subject: [PATCH 10/11] Removed outdated refs to log_and_raise from how to setup model docs --- docs/source/development/defining_new_models.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/source/development/defining_new_models.md b/docs/source/development/defining_new_models.md index d9381a740..410cf3c1c 100644 --- a/docs/source/development/defining_new_models.md +++ b/docs/source/development/defining_new_models.md @@ -26,7 +26,7 @@ touch virtual_rainforest/models/freshwater/model.py ``` This script must import a number of things to be able to set up a new `Model` class -correctly. It's worth noting that `log_and_raise` will be removed soon. +correctly. ```python # One of the member functions of the Model class returns a class instance. mypy doesn't @@ -42,7 +42,7 @@ import pint from numpy import datetime64, timedelta64 # Logging of relevant information handled by Virtual Rainforest logger module -from virtual_rainforest.core.logger import LOGGER, log_and_raise +from virtual_rainforest.core.logger import LOGGER # New model class will inherit from BaseModel. # InitialisationError is a custom exception, for case where a `Model` class cannot be # properly initialised based on the data contained in the configuration @@ -86,10 +86,11 @@ def __init__( # Sanity checking of input variables goes here if no_of_ponds < 0: - log_and_raise( - "There has to be at least one pond in the freshwater model!", - InitialisationError, - ) + to_raise = InitialisationError( + "There has to be at least one pond in the freshwater model!" + ) + LOGGER.critical(to_raise) + raise to_raise # Provide general attributes to the __init__ of the base class super().__init__(update_interval, start_time, **kwargs) From 5ec989292d8f0e68a29ae69260be2fa1e24cf8aa Mon Sep 17 00:00:00 2001 From: Jacob Cook Date: Thu, 2 Feb 2023 09:56:55 +0000 Subject: [PATCH 11/11] Removed hacky try except block from around parse_args --- virtual_rainforest/entry_points.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/virtual_rainforest/entry_points.py b/virtual_rainforest/entry_points.py index 9d7d31b23..aa7ecdf0d 100644 --- a/virtual_rainforest/entry_points.py +++ b/virtual_rainforest/entry_points.py @@ -55,16 +55,7 @@ def _vr_run_cli() -> None: version="%(prog)s {version}".format(version=vr.__version__), ) - try: - args = parser.parse_args() - except SystemExit as excep: - if excep.code != 0: - # Catch unexpected args and similar problems - exit = SystemExit("For more information please run vr_run --help") - LOGGER.critical(exit) - raise exit - else: - raise excep + args = parser.parse_args() if not args.cfg_paths: to_raise = ConfigurationError(