Fixing config parsing

docs/source/_static/vr_full_model_configuration.toml → docs/source/_static/ve_full_model_configuration.toml

@@ -2,7 +2,7 @@
 out_continuous_file_name = "all_continuous_data.nc"
 out_final_file_name = "final_state.nc"
 out_initial_file_name = "initial_state.nc"
-out_merge_file_name = "vr_full_model_configuration.toml"
+out_merge_file_name = "ve_full_model_configuration.toml"
 out_path = "/tmp/ve_example/out"
 save_continuous_data = true
 save_final_state = true

docs/source/api/core/variables.md

@@ -9,9 +9,19 @@ jupytext:
     format_version: 0.13
     jupytext_version: 1.17.1
 kernelspec:
-  display_name: vr_python3
+  display_name: Python 3 (ipykernel)
   language: python
-  name: vr_python3
+  name: python3
+language_info:
+  codemirror_mode:
+    name: ipython
+    version: 3
+  file_extension: .py
+  mimetype: text/x-python
+  name: python
+  nbconvert_exporter: python
+  pygments_lexer: ipython3
+  version: 3.11.9
 ---
 
 # API documentation for the {mod}`~virtual_ecosystem.core.variables` module

docs/source/using_the_ve/configuration/config.md

@@ -28,14 +28,21 @@ language_info:
 This module is used to configure a `virtual_ecosystem` simulation run. This module
 reads in a set of configuration files written using `toml`. It is setup in such a way as
 to allow a reduced set of modules to be configured (e.g. just `plants` and `soil`), and
-to allow specific module implementations to be configured (e.g. `plants_with_hydro`
-instead of `plants`). The resulting combined model configuration is validated against a
-set of [`JSON Schema`](https://json-schema.org). If this passes, a combined output file is
-saved as a permanent record of the model configuration. This configuration is also saved
-as a dictionary accessible to other modules and scripts.
+to allow specific module implementations to be configured (e.g. `abiotic_simple`
+instead of `abiotic`). It deliberately accepts multiple configuration files in order to
+allow users to maintain a library of model configuration files that can be used within
+multiple different simulations.
+
+When the run starts, the configuration inputs are combined and the resulting combined
+model configuration is validated. By default, the combined configuration is written out
+to a single file to provide a permanent record of the model configuration. All file
+paths within the combined configuration are converted to absolute paths to ensure that
+input paths across the initial configurations can be located from within the combined
+configuration - this does tie the combined configuration paths to the file system in
+which the simulation is run.
 
 ::::{dropdown} An example configuration file
-:::{literalinclude} ../../_static/vr_full_model_configuration.toml
+:::{literalinclude} ../../_static/ve_full_model_configuration.toml
 :language: toml
 :::
 ::::

tests/core/test_config.py

@@ -764,8 +764,8 @@ def test_Config_export_config(caplog, shared_datadir, auto, expected_log_entries
                 "baz": 6,
             },
             {
-                "file1_path": "path/to/config/file.txt",
-                "other_path": "path/to/config/file2.txt",
+                "file1_path": str(Path("path/to/config/file.txt").absolute()),
+                "other_path": str(Path("path/to/config/file2.txt").absolute()),
                 "foo": "bar",
                 "baz": 6,
             },
@@ -779,8 +779,8 @@ def test_Config_export_config(caplog, shared_datadir, auto, expected_log_entries
                 "baz": 6,
             },
             {
-                "file1_path": "path/to/config/data/file.txt",
-                "other_path": "path/to/config/data/file2.txt",
+                "file1_path": str(Path("path/to/config/data/file.txt").absolute()),
+                "other_path": str(Path("path/to/config/data/file2.txt").absolute()),
                 "foo": "bar",
                 "baz": 6,
             },
@@ -794,8 +794,8 @@ def test_Config_export_config(caplog, shared_datadir, auto, expected_log_entries
                 "baz": 6,
             },
             {
-                "file1_path": "path/to/data/file.txt",
-                "other_path": "path/to/data/file2.txt",
+                "file1_path": str(Path("path/to/data/file.txt").absolute()),
+                "other_path": str(Path("path/to/data/file2.txt").absolute()),
                 "foo": "bar",
                 "baz": 6,
             },
@@ -809,8 +809,8 @@ def test_Config_export_config(caplog, shared_datadir, auto, expected_log_entries
                 "baz": 6,
             },
             {
-                "file1_path": "path/data/file.txt",
-                "other_path": "path/data/file2.txt",
+                "file1_path": str(Path("path/data/file.txt").absolute()),
+                "other_path": str(Path("path/data/file2.txt").absolute()),
                 "foo": "bar",
                 "baz": 6,
             },
@@ -824,8 +824,8 @@ def test_Config_export_config(caplog, shared_datadir, auto, expected_log_entries
                 "baz": 6,
             },
             {
-                "file1_path": "path/data/file.txt",
-                "other_path": "path/to/config/data/file2.txt",
+                "file1_path": str(Path("path/data/file.txt").absolute()),
+                "other_path": str(Path("path/to/config/data/file2.txt").absolute()),
                 "foo": "bar",
                 "baz": 6,
             },
@@ -834,17 +834,17 @@ def test_Config_export_config(caplog, shared_datadir, auto, expected_log_entries
     ),
 )
 def test__resolve_config_paths_file_locations(
-    tmpdir, cfg_is_relative, filepath_is_relative, params_dict_source, expected
+    cfg_is_relative, filepath_is_relative, params_dict_source, expected
 ):
-    """Test the __resolve_config_paths() function can get relative paths correctly.
+    """Test the __resolve_config_paths() function handles file resolution correctly.
 
-    This is using tmpdir to get an OS appropriate base file path - the location is not
-    used for any actual file IO.
+    This test uses the test execution directory as an OS appropriate base file path,
+    the location is not used for any actual file IO.
     """
     from virtual_ecosystem.core.config import _resolve_config_paths
 
     # Get the config path to be used
-    execution_root = Path(tmpdir)
+    execution_root = Path()
     cfg_relative = Path("path/to/config")
     cfg_absolute = execution_root / cfg_relative
     cfg_path = cfg_relative if cfg_is_relative else cfg_absolute
@@ -856,9 +856,7 @@ def test__resolve_config_paths_file_locations(
     if not filepath_is_relative:
         for key, val in params_dict.items():
             if key.endswith("_path"):
-                params_dict[key] = str(
-                    (execution_root / cfg_relative / Path(val)).resolve()
-                )
+                params_dict[key] = str((cfg_absolute / Path(val)).resolve())
 
     # Run the function
     _resolve_config_paths(cfg_path, params_dict)
@@ -867,11 +865,7 @@ def test__resolve_config_paths_file_locations(
         # Test that paths have been resolved as expected
         # but that the other entries have been left alone
         if key.endswith("_path"):
-            if cfg_is_relative and filepath_is_relative:
-                assert Path(val) == Path(expected[key])
-            else:
-                assert Path(val) == execution_root / expected[key]
-
+            assert Path(val) == Path(expected[key])
         elif key == "foo":
             assert val == "bar"
         elif key == "baz":
@@ -880,8 +874,9 @@ def test__resolve_config_paths_file_locations(
 
 @pytest.mark.parametrize(
     "params_dict,raises,expected,err_msg",
-    # The str(Path(x)) pattern in the expected values below is to ensure that
-    # the expected paths are converted to the file system of the test machine.
+    # The str(Path(x).absolute()) pattern in the expected values below is to ensure that
+    # the expected paths are converted to the file system of the test machine and
+    # converted to absolute paths within the test execution directory
     (
         pytest.param(
             {
@@ -892,8 +887,8 @@ def test__resolve_config_paths_file_locations(
             },
             does_not_raise(),
             {
-                "file1_path": str(Path("path/to/config/file.txt")),
-                "other_path": str(Path("path/to/config/file2.txt")),
+                "file1_path": str(Path("path/to/config/file.txt").absolute()),
+                "other_path": str(Path("path/to/config/file2.txt").absolute()),
                 "foo": "bar",
                 "baz": 6,
             },
@@ -909,8 +904,8 @@ def test__resolve_config_paths_file_locations(
             },
             does_not_raise(),
             {
-                "file1_path": str(Path("path/to/config/file.txt")),
-                "other_path": str(Path("path/file2.txt")),
+                "file1_path": str(Path("path/to/config/file.txt").absolute()),
+                "other_path": str(Path("path/file2.txt").absolute()),
                 "foo": "bar",
                 "baz": 6,
             },
@@ -925,8 +920,11 @@ def test__resolve_config_paths_file_locations(
             },
             does_not_raise(),
             {
-                "file1_path": str(Path("path/to/config/file.txt")),
-                "nested": {"other_path": str(Path("path/file2.txt")), "foo": "bar"},
+                "file1_path": str(Path("path/to/config/file.txt").absolute()),
+                "nested": {
+                    "other_path": str(Path("path/file2.txt").absolute()),
+                    "foo": "bar",
+                },
                 "baz": 6,
             },
             None,

tests/core/test_logger.py

@@ -48,7 +48,7 @@ def test_add_file_logger():
 
         # Check the handler has been created by looking for it by name - this will raise
         # with StopIteration if it fails.
-        _ = next(handler for handler in LOGGER.handlers if handler.name == "vr_logfile")
+        _ = next(handler for handler in LOGGER.handlers if handler.name == "ve_logfile")
 
         # Check the file is being written to
         assert tempfile.exists()
@@ -98,7 +98,7 @@ def test_remove_file_logger(caplog):
 
         with pytest.raises(StopIteration):
             _ = next(
-                handler for handler in LOGGER.handlers if handler.name == "vr_logfile"
+                handler for handler in LOGGER.handlers if handler.name == "ve_logfile"
             )
 
         # Check the logging works

virtual_ecosystem/__init__.py

@@ -1,6 +1,9 @@
 """The base initialisation of the Virtual Ecosystem model."""
 
 import importlib.metadata
+import warnings
+
+from pyrealm.core.experimental import ExperimentalFeatureWarning
 
 from . import example_data
 
@@ -13,3 +16,10 @@
 circumstances, e.g. if this Python package is inside a zip file, but it should work in
 the ordinary case.
 """
+
+# Ignore experimental and user warnings coming from pyrealm
+
+warnings.filterwarnings(
+    action="ignore", category=ExperimentalFeatureWarning, module=r"pyrealm"
+)
+warnings.filterwarnings(action="ignore", category=UserWarning, module=r"pyrealm")

virtual_ecosystem/core/config.py

@@ -105,8 +105,7 @@ def _resolve_config_paths(config_dir: Path, config_dict: dict[str, Any]) -> None
     configuration file. This becomes a problem when configurations are compiled across
     multiple configuration files, possibly in different locations, so this function
     searches the configuration dictionary loaded from a single file and updates
-    configure paths to be congruent from the directory in which Virtual Ecosystem is
-    being run.
+    configured relative paths to their absolute paths.
 
     At present, the configuration schema does not have an explicit mechanism to type a
     configuration option as being a path, so we currently use the `_path` suffix to
@@ -123,12 +122,16 @@ def _resolve_config_paths(config_dir: Path, config_dict: dict[str, Any]) -> None
         ValueError: if a key ending in ``_path`` has a non-string value.
     """
 
+    if not config_dir.is_absolute():
+        config_dir = config_dir.absolute()
+
     for key, item in config_dict.items():
         if isinstance(item, dict):
             _resolve_config_paths(config_dir=config_dir, config_dict=item)
         elif isinstance(item, list):
             for list_entry in item:
-                _resolve_config_paths(config_dir=config_dir, config_dict=list_entry)
+                if isinstance(list_entry, dict):
+                    _resolve_config_paths(config_dir=config_dir, config_dict=list_entry)
         elif key.endswith("_path"):
             if not isinstance(item, str):
                 raise ValueError(
@@ -137,15 +140,9 @@ def _resolve_config_paths(config_dir: Path, config_dict: dict[str, Any]) -> None
             file_path = Path(item)
             if not file_path.is_absolute():
                 # The resolve method is used here because it is the only method to
-                # resolve ../ entries from relative file paths. However, it also makes
-                # all paths absolute, which lengthens paths if the config directory
-                # itself is relative. The approach here converts `path/to/config` and
-                # `../data/file1.nc` to `path/to/data/file1.nc` when both paths are
-                # relative
-                file_resolved = (config_dir / file_path).resolve()
-                if not config_dir.is_absolute():
-                    config_absolute = Path(config_dir.root).absolute()
-                    file_resolved = file_resolved.relative_to(config_absolute)
+                # resolve ../ entries from relative file paths and then the path is made
+                # explicitly absolute
+                file_resolved = (config_dir / file_path).resolve().absolute()
 
                 config_dict[key] = str(file_resolved)
 

virtual_ecosystem/core/logger.py

@@ -90,7 +90,7 @@
 def add_file_logger(logfile: Path) -> None:
     """Redirect logging to a provided file path.
 
-    This function adds a FileHandler with the name ``vr_logfile`` to
+    This function adds a FileHandler with the name ``ve_logfile`` to
     :data:`~virtual_ecosystem.core.logger.LOGGER` using the provided ``logfile`` path.
     It also turns off record propagation so that logging messages are only sent to that
     file and not to the parent StreamHandler.
@@ -104,7 +104,7 @@ def add_file_logger(logfile: Path) -> None:
     """
 
     for handler in LOGGER.handlers:
-        if isinstance(handler, logging.FileHandler) and handler.name == "vr_logfile":
+        if isinstance(handler, logging.FileHandler) and handler.name == "ve_logfile":
             raise RuntimeError(f"Already logging to file: {handler.baseFilename}")
 
     # Do not propogate errors up to parent handler - this avoids mirroring the log
@@ -116,30 +116,30 @@ def add_file_logger(logfile: Path) -> None:
     formatter = logging.Formatter(fmt=format)
     handler = logging.FileHandler(logfile)
     handler.setFormatter(formatter)
-    handler.name = "vr_logfile"
+    handler.name = "ve_logfile"
     LOGGER.addHandler(handler)
     LOGGER.setLevel(logging.DEBUG)
 
 
 def remove_file_logger() -> None:
     """Remove the file logger and return to stream logging.
 
-    This function attempts to remove the ``vr_logfile`` FileHandler that is added by
+    This function attempts to remove the ``ve_logfile`` FileHandler that is added by
     :func:`~virtual_ecosystem.core.logger.add_file_logger`. If that file handler is
     not found it simple exits, otherwise it removes the file handler and restores
     message propagation.
     """
 
     try:
         # Find the file logger by name and remove it
-        vr_logfile = next(
-            handler for handler in LOGGER.handlers if handler.name == "vr_logfile"
+        ve_logfile = next(
+            handler for handler in LOGGER.handlers if handler.name == "ve_logfile"
         )
     except StopIteration:
         return
 
-    vr_logfile.close()
-    LOGGER.removeHandler(vr_logfile)
+    ve_logfile.close()
+    LOGGER.removeHandler(ve_logfile)
 
     # Allow logger messages to propogate back down to the root StreamHandler
     LOGGER.propagate = True

virtual_ecosystem/core/module_schema.json

@@ -167,7 +167,7 @@
                   "out_merge_file_name": {
                      "description": "Name for TOML file containing merged configs",
                      "type": "string",
-                     "default": "vr_full_model_configuration.toml",
+                     "default": "ve_full_model_configuration.toml",
                      "pattern": "^[^/\\\\]+$"
                   }
                },

virtual_ecosystem/entry_points.py

@@ -128,8 +128,11 @@ def ve_run_cli(args_list: list[str] | None = None) -> int:
 
     The resolved complete configuration will then be written to a single consolidated
     config file in the output path with a default name of
-    `vr_full_model_configuration.toml`. This can be disabled by setting the
-    `core.data_output_options.save_merged_config` option to false.
+    `ve_full_model_configuration.toml`. This can be disabled by setting the
+    `core.data_output_options.save_merged_config` option to false. Note that the merged
+    configuration automatically converts all file paths within the merged configurations
+    to absolute file paths- this ties the merged configuration to the file system where
+    the run is executed.
 
     Args:
         args_list: This is a developer and testing facing argument that is used to