From b504b62d44a9af8b1ad7357424398a8ed1f2fbe0 Mon Sep 17 00:00:00 2001 From: Drew Oldag <47493171+drewoldag@users.noreply.github.com> Date: Thu, 24 Oct 2024 14:54:26 -0700 Subject: [PATCH] Handle externally defined default configuration files (#100) * WIP - Adding a ConfigManager class to help wrangle the various defaults that could be imported. * Responding to comments, fixing tests, cleaning up a bit. * The check should be on the value, not on the key. --- src/fibad/__init__.py | 4 +- src/fibad/config_utils.py | 289 +++++++++++++++++-------------- src/fibad/fibad.py | 12 +- tests/fibad/test_config_utils.py | 8 +- 4 files changed, 169 insertions(+), 144 deletions(-) diff --git a/src/fibad/__init__.py b/src/fibad/__init__.py index 37f2c22..4f06054 100644 --- a/src/fibad/__init__.py +++ b/src/fibad/__init__.py @@ -1,10 +1,8 @@ -from .config_utils import get_runtime_config, log_runtime_config, merge_configs +from .config_utils import log_runtime_config from .fibad import Fibad from .plugin_utils import get_or_load_class, import_module_from_string, update_registry __all__ = [ - "get_runtime_config", - "merge_configs", "log_runtime_config", "get_or_load_class", "import_module_from_string", diff --git a/src/fibad/config_utils.py b/src/fibad/config_utils.py index 9e447e3..095b75f 100644 --- a/src/fibad/config_utils.py +++ b/src/fibad/config_utils.py @@ -1,4 +1,5 @@ import datetime +import importlib import logging from pathlib import Path from typing import Union @@ -62,76 +63,171 @@ def clear(self): raise RuntimeError("Removing keys or sections from a ConfigDict using clear() is not supported") -def validate_runtime_config(runtime_config: ConfigDict): - """Validates that defaults exist for every config value before we begin to use a config. - - This should be called at the moment the runtime config is fully baked for science calculations. Meaning - that all sources of config info have been combined in `runtime_config` and there are no further - config altering operations that will be performed. - - Parameters - ---------- - runtime_config : ConfigDict - The current runtime config dictionary. - - Raises - ------ - RuntimeError - Raised if any config that exists in the runtime config does not have a default defined +class ConfigManager: + """A class to manage the runtime configuration for a Fibad object. This class + will contain all the logic and methods for reading, merging, and validating + the runtime configuration. """ - default_config = _read_runtime_config(DEFAULT_CONFIG_FILEPATH) - _validate_runtime_config(runtime_config, default_config) + def __init__( + self, + runtime_config_filepath: Union[Path, str] = None, + default_config_filepath: Union[Path, str] = DEFAULT_CONFIG_FILEPATH, + ): + self.fibad_default_config = self._read_runtime_config(default_config_filepath) -def _validate_runtime_config(runtime_config: ConfigDict, default_config: ConfigDict): - """Recursive helper for validate_runtime_config. + self.runtime_config_filepath = runtime_config_filepath + if self.runtime_config_filepath is None: + self.user_specific_config = ConfigDict() + else: + self.user_specific_config = self._read_runtime_config(self.runtime_config_filepath) + + self.external_library_config_paths = self._find_external_library_default_config_paths( + self.user_specific_config + ) + + self.overall_default_config = {} + self._merge_defaults() + + self.config = self.merge_configs(self.overall_default_config, self.user_specific_config) + self._validate_runtime_config(self.config, self.overall_default_config) + + @staticmethod + def _read_runtime_config(config_filepath: Union[Path, str] = DEFAULT_CONFIG_FILEPATH) -> ConfigDict: + """Read a single toml file and return a ConfigDict + + Parameters + ---------- + config_filepath : Union[Path, str], optional + The path to the config file, by default DEFAULT_CONFIG_FILEPATH + + Returns + ------- + ConfigDict + The contents of the toml file as a ConfigDict + """ + config_filepath = Path(config_filepath) + parsed_dict = {} + if config_filepath.exists(): + with open(config_filepath, "r") as f: + parsed_dict = toml.load(f) - The two arguments passed in must represent the same nesting level of the runtime config and all - default config parameters respectively. + return ConfigDict(parsed_dict) - Parameters - ---------- - runtime_config : ConfigDict - Nested config dictionary representing the runtime config. - default_config : ConfigDict - Nested config dictionary representing the defaults - - Raises - ------ - RuntimeError - Raised if any config that exists in the runtime config does not have a default defined in - default_config - """ - for key in runtime_config: - if key not in default_config: - msg = f"Runtime config contains key or section {key} which has no default defined. " - msg += f"All configuration keys and sections must be defined in {DEFAULT_CONFIG_FILEPATH}" - raise RuntimeError(msg) - - if isinstance(runtime_config[key], dict): - if not isinstance(default_config[key], dict): - msg = f"Runtime config contains a section named {key} which is the name of a value in the " - msg += "default config. Please choose another name for this section." + @staticmethod + def _find_external_library_default_config_paths(runtime_config: dict) -> set: + """Search for external libraries in the runtime configuration and gather the + libpath specifications so that we can load the default configs for the libraries. + + Parameters + ---------- + runtime_config : dict + The runtime configuration. + Returns + ------- + set + A tuple containing the default configuration Paths for the external + libraries that are requested in the users configuration file. + """ + + default_configs = set() + for key, value in runtime_config.items(): + if isinstance(value, dict): + default_configs |= ConfigManager._find_external_library_default_config_paths(value) + else: + if key == "name" and "." in value: + external_library = value.split(".")[0] + if importlib.util.find_spec(external_library) is not None: + try: + lib = importlib.import_module(external_library) + lib_default_config_path = Path(lib.__file__).parent / "default_config.toml" + if lib_default_config_path.exists(): + default_configs.add(lib_default_config_path) + except ModuleNotFoundError: + logger.error( + f"External library {lib} not found. Please install it before running." + ) + raise + + return default_configs + + def _merge_defaults(self): + """Merge the default configurations from the fibad and external libraries.""" + + # Merge all external library default configurations first + for path in self.external_library_config_paths: + external_library_config = self._read_runtime_config(path) + self.overall_default_config = self.merge_configs( + self.overall_default_config, external_library_config + ) + + # Merge the external library default configurations with the fibad default configuration + self.overall_default_config = self.merge_configs( + self.fibad_default_config, self.overall_default_config + ) + + @staticmethod + def merge_configs(default_config: dict, overriding_config: dict) -> dict: + """Merge two configurations dictionaries with the overriding_config values + overriding the default_config values. + + Parameters + ---------- + default_config : dict + The default configuration. + overriding_config : dict + The user defined configuration. + + Returns + ------- + dict + The merged configuration. + """ + + final_config = default_config.copy() + for k, v in overriding_config.items(): + if k in final_config and isinstance(final_config[k], dict) and isinstance(v, dict): + final_config[k] = ConfigManager.merge_configs(default_config[k], v) + else: + final_config[k] = v + + return final_config + + @staticmethod + def _validate_runtime_config(runtime_config: ConfigDict, default_config: ConfigDict): + """Recursive helper to check that all keys in runtime_config have a default + in the merged default_config. + + The two arguments passed in must represent the same nesting level of the + runtime config and all default config parameters respectively. + + Parameters + ---------- + runtime_config : ConfigDict + Nested config dictionary representing the runtime config. + default_config : ConfigDict + Nested config dictionary representing the defaults + + Raises + ------ + RuntimeError + Raised if any config that exists in the runtime config does not have a default defined in + default_config + """ + for key in runtime_config: + if key not in default_config: + msg = f"Runtime config contains key or section {key} which has no default defined. " + msg += f"All configuration keys and sections must be defined in {DEFAULT_CONFIG_FILEPATH}" raise RuntimeError(msg) - _validate_runtime_config(runtime_config[key], default_config[key]) - -def _read_runtime_config(config_filepath: Union[Path, str] = DEFAULT_CONFIG_FILEPATH) -> ConfigDict: - """Read a single toml file and return a config dictionary - - Parameters - ---------- - config_filepath : Union[Path, str], optional - What file is to be read, by default DEFAULT_CONFIG_FILEPATH - - Returns - ------- - ConfigDict - The contents of that toml file as nested ConfigDicts - """ - with open(config_filepath, "r") as f: - parsed_dict = toml.load(f) - return ConfigDict(parsed_dict) + if isinstance(runtime_config[key], dict): + if not isinstance(default_config[key], dict): + msg = ( + f"Runtime config contains a section named {key} which is the name of a value in the " + ) + msg += "default config. Please choose another name for this section." + raise RuntimeError(msg) + ConfigManager._validate_runtime_config(runtime_config[key], default_config[key]) def resolve_runtime_config(runtime_config_filepath: Union[Path, str, None] = None) -> Path: @@ -167,73 +263,6 @@ def resolve_runtime_config(runtime_config_filepath: Union[Path, str, None] = Non return runtime_config_filepath -def get_runtime_config( - runtime_config_filepath: Union[Path, str, None] = None, - default_config_filepath: Union[Path, str] = DEFAULT_CONFIG_FILEPATH, -) -> dict: - """This function will load the default runtime configuration file, as well - as the user defined runtime configuration file. - - The two configurations will be merged with values in the user defined config - overriding the values of the default configuration. - - The final merged config will be returned as a dictionary and saved as a file - in the results directory. - - Parameters - ---------- - runtime_config_filepath : Union[Path, str, None] - The path to the runtime configuration file. - default_config_filepath : Union[Path, str] - The path to the default runtime configuration file. - - Returns - ------- - dict - The parsed runtime configuration. - """ - - runtime_config_filepath = resolve_runtime_config(runtime_config_filepath) - default_runtime_config = _read_runtime_config(default_config_filepath) - - if runtime_config_filepath is not DEFAULT_CONFIG_FILEPATH: - if not runtime_config_filepath.exists(): - raise FileNotFoundError(f"Runtime configuration file not found: {runtime_config_filepath}") - users_runtime_config = _read_runtime_config(runtime_config_filepath) - final_runtime_config = merge_configs(default_runtime_config, users_runtime_config) - else: - final_runtime_config = default_runtime_config - - return final_runtime_config - - -def merge_configs(default_config: dict, user_config: dict) -> dict: - """Merge two configurations dictionaries with the user_config values overriding - the default_config values. - - Parameters - ---------- - default_config : dict - The default configuration. - user_config : dict - The user defined configuration. - - Returns - ------- - dict - The merged configuration. - """ - - final_config = default_config.copy() - for k, v in user_config.items(): - if k in final_config and isinstance(final_config[k], dict) and isinstance(v, dict): - final_config[k] = merge_configs(default_config[k], v) - else: - final_config[k] = v - - return final_config - - def create_results_dir(config: ConfigDict, postfix: Union[Path, str]) -> Path: """Creates a results directory for this run. diff --git a/src/fibad/fibad.py b/src/fibad/fibad.py index fa5097d..23dd6c5 100644 --- a/src/fibad/fibad.py +++ b/src/fibad/fibad.py @@ -3,7 +3,7 @@ from pathlib import Path from typing import Union -from .config_utils import get_runtime_config, resolve_runtime_config, validate_runtime_config +from .config_utils import ConfigManager, resolve_runtime_config class Fibad: @@ -43,12 +43,8 @@ def __init__(self, *, config_file: Union[Path, str] = None, setup_logging: bool - You have a single Fibad object in use at any time. This is true in most notebook like environments. """ - self.config = get_runtime_config(runtime_config_filepath=config_file) - - # TODO: For now this is part of Fibad.__init__() In future when external modules can define their own - # configuration keys and associated default values, this will need to occur after those external - # modules have been resolved and loaded. This is why it is separate from get_runtime_config() - validate_runtime_config(self.config) + self.config_manager = ConfigManager(runtime_config_filepath=config_file) + self.config = self.config_manager.config # Configure our logger. We do not use __name__ here because that would give us a "fibad.fibad" logger # which would not aggregate logs from fibad.downloadCutout which creates its own @@ -95,7 +91,7 @@ def __init__(self, *, config_file: Union[Path, str] = None, setup_logging: bool self.logger.info(f"Runtime Config read from: {resolve_runtime_config(config_file)}") def _initialize_log_handlers(self): - """Private initialization helper, Adds handlers and level setting sto the global self.logger object""" + """Private initialization helper, Adds handlers and level setting to the global self.logger object""" general_config = self.config["general"] # default to warning level diff --git a/tests/fibad/test_config_utils.py b/tests/fibad/test_config_utils.py index 58a887a..9876ac4 100644 --- a/tests/fibad/test_config_utils.py +++ b/tests/fibad/test_config_utils.py @@ -1,6 +1,6 @@ import os -from fibad.config_utils import get_runtime_config, merge_configs +from fibad.config_utils import ConfigManager def test_merge_configs(): @@ -27,7 +27,7 @@ def test_merge_configs(): expected = {"a": 5, "b": 2, "c": {"d": 6, "e": 4}, "f": 7} - assert merge_configs(default_config, user_config) == expected + assert ConfigManager.merge_configs(default_config, user_config) == expected def test_get_runtime_config(): @@ -37,7 +37,7 @@ def test_get_runtime_config(): """ this_file_dir = os.path.dirname(os.path.abspath(__file__)) - runtime_config = get_runtime_config( + config_manager = ConfigManager( runtime_config_filepath=os.path.abspath( os.path.join(this_file_dir, "./test_data/test_user_config.toml") ), @@ -46,6 +46,8 @@ def test_get_runtime_config(): ), ) + runtime_config = config_manager.config + expected = { "general": {"use_gpu": False}, "train": {