Merge pull request #158 from ImperialCollegeLondon/feature/kill_log_a…

…nd_raise

Removing `log_and_raise`

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)

virtual_rainforest/core/axes.py

@@ -51,7 +51,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):
@@ -207,6 +207,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:
@@ -226,24 +227,28 @@ 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
             this_validator = validator_found[0]
             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__
 

virtual_rainforest/core/config.py

@@ -44,13 +44,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, Generator, Iterator, Optional, Union
 
 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
@@ -67,9 +67,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.
 
@@ -91,10 +93,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(
@@ -149,31 +152,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()
 
@@ -232,25 +242,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
 
@@ -289,24 +303,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
 
@@ -341,11 +360,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:
@@ -354,7 +374,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()))
@@ -381,10 +403,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(
@@ -414,22 +437,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
 
@@ -460,11 +485,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

virtual_rainforest/core/data.py

@@ -125,7 +125,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
 
 
@@ -139,13 +139,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."""
@@ -185,12 +190,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}'")