Way back when, we added code to resolve paths in configuration files. This is partly so that relative paths are always resolved relative to the specific config TOML they come from, and partly because VE needs resolved paths. This was implemented in #269 - but that PR only handles resolution of file paths in [core.data.variable] blocks and specifically notes that this is something we may need to revisit.
That time is now - the PlantsModel needs to load PFT definitions and forcing that through the data object is extremely heavy handed and will create a bunch of single use constant "variables" floating around in the data object.
The issue here is that the config file values from TOML have a very limited number of types, so paths are just strings, and we only want to try and resolve the strings that are paths and not other things.
-
A clever approach would be to use something that types path settings in a detectable way. We could add "I am a path" properties to our JSON Schema or possibly replace our schema setup with pydantic, which would be cleaner and more pythonic. The problem though is that we are resolving paths in arbitrary chunks of config before combining and validating them. So it's not clear how we'd match values to the schema.
-
A more pragmatic approach right now is to use the config key name to identify paths. We update the _resolve_config_paths function such that:
- It walks over the entire config file contents,
- looking for
key, value pairs where key.endswith('_path') and isinstance(value, str)
- and updates those values with the existing resolution mechanism in that function.
|
def _resolve_config_paths(config_dir: Path, params: dict[str, Any]) -> None: |
|
"""Resolve paths in a configuration file. |
|
|
|
Takes the path of a directory containing a given configuration file and resolves any |
|
file paths in the configuration file contents, relative to that file location. |
|
|
|
Todo: |
|
At present, this only targets `core.data.variable` configuration entries and may |
|
want to resolve additional paths in the future. |
|
|
|
Args: |
|
config_dir: A folder containing a configuration file. |
|
params: A dictionary of contents of the configuration file, which may contain |
|
file paths to resolve. |
|
""" |
This does:
- involve updating the
[core.data.variable] spec to use file_path and not file, but I think it is worth having a single clear pattern.
- mean we have to avoid using keys ending in
_path for other uses, but that seems reasonable.
Way back when, we added code to resolve paths in configuration files. This is partly so that relative paths are always resolved relative to the specific config TOML they come from, and partly because VE needs resolved paths. This was implemented in #269 - but that PR only handles resolution of file paths in
[core.data.variable]blocks and specifically notes that this is something we may need to revisit.That time is now - the PlantsModel needs to load PFT definitions and forcing that through the
dataobject is extremely heavy handed and will create a bunch of single use constant "variables" floating around in thedataobject.The issue here is that the config file values from TOML have a very limited number of types, so paths are just strings, and we only want to try and resolve the strings that are paths and not other things.
A clever approach would be to use something that types path settings in a detectable way. We could add "I am a path" properties to our JSON Schema or possibly replace our schema setup with
pydantic, which would be cleaner and more pythonic. The problem though is that we are resolving paths in arbitrary chunks of config before combining and validating them. So it's not clear how we'd match values to the schema.A more pragmatic approach right now is to use the config key name to identify paths. We update the
_resolve_config_pathsfunction such that:key, valuepairs wherekey.endswith('_path')andisinstance(value, str)virtual_ecosystem/virtual_ecosystem/core/config.py
Lines 100 to 114 in 4cab25c
This does:
[core.data.variable]spec to usefile_pathand notfile, but I think it is worth having a single clear pattern._pathfor other uses, but that seems reasonable.