From 9eb8cd2d526d1d19c1c7960c4e80ff6b86f68b6b Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Wed, 2 Oct 2024 13:50:22 +0200 Subject: [PATCH 1/3] Add Config.update_from_dirs method --- doc/api/esmvalcore.config.rst | 7 ++ esmvalcore/_main.py | 10 +-- esmvalcore/config/_config_object.py | 56 +++++++++--- tests/unit/config/test_config_object.py | 111 ++++++++++++++++++------ 4 files changed, 141 insertions(+), 43 deletions(-) diff --git a/doc/api/esmvalcore.config.rst b/doc/api/esmvalcore.config.rst index 9b01587263..231140effd 100644 --- a/doc/api/esmvalcore.config.rst +++ b/doc/api/esmvalcore.config.rst @@ -88,6 +88,13 @@ To load the configuration object from custom directories, use: >>> dirs = ['my/default/config', 'my/custom/config'] >>> CFG.load_from_dirs(dirs) +To update the existing configuration object from custom directories, use: + +.. code-block:: python + + >>> dirs = ['my/default/config', 'my/custom/config'] + >>> CFG.update_from_dirs(dirs) + Session ******* diff --git a/esmvalcore/_main.py b/esmvalcore/_main.py index d0bd6fcf10..451f228ae8 100755 --- a/esmvalcore/_main.py +++ b/esmvalcore/_main.py @@ -403,7 +403,6 @@ def run(self, recipe, **kwargs): """ from .config import CFG - from .config._config_object import _get_all_config_dirs from .exceptions import InvalidConfigParameter cli_config_dir = kwargs.pop("config_dir", None) @@ -427,10 +426,9 @@ def run(self, recipe, **kwargs): # New in v2.12.0: read additional configuration directory given by CLI # argument - if CFG.get("config_file") is None: # remove in v2.14.0 - config_dirs = _get_all_config_dirs(cli_config_dir) + if CFG.get("config_file") is None and cli_config_dir is not None: try: - CFG.load_from_dirs(config_dirs) + CFG.update_from_dirs([cli_config_dir]) # Potential errors must come from --config_dir (i.e., # cli_config_dir) since other sources have already been read (and @@ -458,8 +456,8 @@ def run(self, recipe, **kwargs): # New in v2.12.0 else: - config_dirs = _get_all_config_dirs(cli_config_dir) # remove v2.14 - CFG.load_from_dirs(config_dirs) + if cli_config_dir is not None: + CFG.update_from_dirs([cli_config_dir]) @staticmethod def _create_session_dir(session): diff --git a/esmvalcore/config/_config_object.py b/esmvalcore/config/_config_object.py index dfe784ef58..e24be05914 100644 --- a/esmvalcore/config/_config_object.py +++ b/esmvalcore/config/_config_object.py @@ -317,8 +317,17 @@ def load_from_file( self.clear() self.update(Config._load_user_config(filename)) + @staticmethod + def _get_config_dict_from_dirs(dirs: Iterable[str | Path]) -> dict: + """Get configuration :obj:`dict` from directories.""" + dirs_str: list[str] = [] + for config_dir in dirs: + config_dir = Path(config_dir).expanduser().absolute() + dirs_str.append(str(config_dir)) + return dask.config.collect(paths=dirs_str, env={}) + def load_from_dirs(self, dirs: Iterable[str | Path]) -> None: - """Load configuration object from directories. + """Clear and load configuration object from directories. This searches for all YAML files within the given directories and merges them together using :func:`dask.config.collect`. Nested objects @@ -344,23 +353,17 @@ def load_from_dirs(self, dirs: Iterable[str | Path]) -> None: Invalid configuration option given. """ - dirs_str: list[str] = [] - # Always consider default options; these have the lowest priority - dirs_str.append(str(DEFAULT_CONFIG_DIR)) + dirs = [DEFAULT_CONFIG_DIR] + list(dirs) - for config_dir in dirs: - config_dir = Path(config_dir).expanduser().absolute() - dirs_str.append(str(config_dir)) - - new_config_dict = dask.config.collect(paths=dirs_str, env={}) + new_config_dict = self._get_config_dict_from_dirs(dirs) self.clear() self.update(new_config_dict) self.check_missing() def reload(self) -> None: - """Reload the configuration object. + """Clear and reload the configuration object. This will read all YAML files in the user configuration directory (by default ``~/.config/esmvaltool``, but this can be changed with the @@ -431,6 +434,39 @@ def start_session(self, name: str) -> Session: session = Session(config=self.copy(), name=name) return session + def update_from_dirs(self, dirs: Iterable[str | Path]) -> None: + """Update configuration object from directories. + + This will first search for all YAML files within the given directories + and merge them together using :func:`dask.config.collect` (values in + the latter directories are preferred to those in the former). Then, the + current configuration is merged with these new configuration options + using :func:`dask.config.merge` (new values are preferred over old + values). Nested objects are properly considered; see + :func:`dask.config.update` for details. + + Note + ---- + Just like :func:`dask.config.collect`, this silently ignores + non-existing directories. + + Parameters + ---------- + dirs: + A list of directories to search for YAML configuration files. + + Raises + ------ + esmvalcore.exceptions.InvalidConfigParameter + Invalid configuration option given. + + """ + new_config_dict = self._get_config_dict_from_dirs(dirs) + merged_config_dict = dask.config.merge(self, new_config_dict) + self.update(merged_config_dict) + + self.check_missing() + class Session(ValidatedConfig): """Container class for session configuration and directory information. diff --git a/tests/unit/config/test_config_object.py b/tests/unit/config/test_config_object.py index fa6c3111b3..1c3f36b033 100644 --- a/tests/unit/config/test_config_object.py +++ b/tests/unit/config/test_config_object.py @@ -393,33 +393,8 @@ def test_get_global_config_deprecated(mocker, tmp_path): assert cfg["output_dir"] == Path("/new/output/dir") -@pytest.mark.parametrize( - "dirs,output_file_type,rootpath", - [ - ([], "png", {"default": "~/climate_data"}), - (["/this/path/does/not/exist"], "png", {"default": "~/climate_data"}), - (["{tmp_path}/config1"], "1", {"default": "1", "1": "1"}), - ( - ["{tmp_path}/config1", "/this/path/does/not/exist"], - "1", - {"default": "1", "1": "1"}, - ), - ( - ["{tmp_path}/config1", "{tmp_path}/config2"], - "2b", - {"default": "2b", "1": "1", "2": "2b"}, - ), - ( - ["{tmp_path}/config2", "{tmp_path}/config1"], - "1", - {"default": "1", "1": "1", "2": "2b"}, - ), - ], -) -def test_load_from_dirs_always_default( - dirs, output_file_type, rootpath, tmp_path -): - """Test `Config.load_from_dirs`.""" +def _setup_config_dirs(tmp_path): + """Setup test configuration directories.""" config1 = tmp_path / "config1" / "1.yml" config2a = tmp_path / "config2" / "2a.yml" config2b = tmp_path / "config2" / "2b.yml" @@ -456,6 +431,36 @@ def test_load_from_dirs_always_default( ) ) + +@pytest.mark.parametrize( + "dirs,output_file_type,rootpath", + [ + ([], "png", {"default": "~/climate_data"}), + (["/this/path/does/not/exist"], "png", {"default": "~/climate_data"}), + (["{tmp_path}/config1"], "1", {"default": "1", "1": "1"}), + ( + ["{tmp_path}/config1", "/this/path/does/not/exist"], + "1", + {"default": "1", "1": "1"}, + ), + ( + ["{tmp_path}/config1", "{tmp_path}/config2"], + "2b", + {"default": "2b", "1": "1", "2": "2b"}, + ), + ( + ["{tmp_path}/config2", "{tmp_path}/config1"], + "1", + {"default": "1", "1": "1", "2": "2b"}, + ), + ], +) +def test_load_from_dirs_always_default( + dirs, output_file_type, rootpath, tmp_path +): + """Test `Config.load_from_dirs`.""" + _setup_config_dirs(tmp_path) + config_dirs = [] for dir_ in dirs: config_dirs.append(dir_.format(tmp_path=str(tmp_path))) @@ -465,11 +470,14 @@ def test_load_from_dirs_always_default( cfg = Config() assert not cfg + cfg["rootpath"] = {"X": "x"} + cfg["search_esgf"] = "when_missing" cfg.load_from_dirs(config_dirs) assert cfg["output_file_type"] == output_file_type assert cfg["rootpath"] == rootpath + assert cfg["search_esgf"] == "never" @pytest.mark.parametrize( @@ -514,3 +522,52 @@ def test_get_all_config_sources(cli_config_dir, output, monkeypatch): cli_config_dir ) assert config_srcs == output + + +@pytest.mark.parametrize( + "dirs,output_file_type,rootpath", + [ + ([], None, {"X": "x"}), + (["/this/path/does/not/exist"], None, {"X": "x"}), + (["{tmp_path}/config1"], "1", {"default": "1", "1": "1", "X": "x"}), + ( + ["{tmp_path}/config1", "/this/path/does/not/exist"], + "1", + {"default": "1", "1": "1", "X": "x"}, + ), + ( + ["{tmp_path}/config1", "{tmp_path}/config2"], + "2b", + {"default": "2b", "1": "1", "2": "2b", "X": "x"}, + ), + ( + ["{tmp_path}/config2", "{tmp_path}/config1"], + "1", + {"default": "1", "1": "1", "2": "2b", "X": "x"}, + ), + ], +) +def test_update_from_dirs(dirs, output_file_type, rootpath, tmp_path): + """Test `Config.update_from_dirs`.""" + _setup_config_dirs(tmp_path) + + config_dirs = [] + for dir_ in dirs: + config_dirs.append(dir_.format(tmp_path=str(tmp_path))) + for name, path in rootpath.items(): + path = Path(path).expanduser().absolute() + rootpath[name] = [path] + + cfg = Config() + assert not cfg + cfg["rootpath"] = {"X": "x"} + cfg["search_esgf"] = "when_missing" + + cfg.update_from_dirs(config_dirs) + + if output_file_type is None: + assert "output_file_type" not in cfg + else: + assert cfg["output_file_type"] == output_file_type + assert cfg["rootpath"] == rootpath + assert cfg["search_esgf"] == "when_missing" From d2cde6d01b0952ac9f07979257058d17694dad3a Mon Sep 17 00:00:00 2001 From: Manuel Schlund <32543114+schlunma@users.noreply.github.com> Date: Wed, 2 Oct 2024 15:02:12 +0200 Subject: [PATCH 2/3] Update esmvalcore/config/_config_object.py Co-authored-by: Bouwe Andela --- esmvalcore/config/_config_object.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esmvalcore/config/_config_object.py b/esmvalcore/config/_config_object.py index e24be05914..b8ab2a68b1 100644 --- a/esmvalcore/config/_config_object.py +++ b/esmvalcore/config/_config_object.py @@ -438,8 +438,9 @@ def update_from_dirs(self, dirs: Iterable[str | Path]) -> None: """Update configuration object from directories. This will first search for all YAML files within the given directories - and merge them together using :func:`dask.config.collect` (values in - the latter directories are preferred to those in the former). Then, the + and merge them together using :func:`dask.config.collect` (if identical values + are provided in multiple files, the value from the last file will be used). + Then, the current configuration is merged with these new configuration options using :func:`dask.config.merge` (new values are preferred over old values). Nested objects are properly considered; see From 5cc97894556d0a456cabc613ad648ba61c7ac04d Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Wed, 2 Oct 2024 15:02:55 +0200 Subject: [PATCH 3/3] Flake8 --- esmvalcore/config/_config_object.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/esmvalcore/config/_config_object.py b/esmvalcore/config/_config_object.py index b8ab2a68b1..baa344f829 100644 --- a/esmvalcore/config/_config_object.py +++ b/esmvalcore/config/_config_object.py @@ -438,13 +438,12 @@ def update_from_dirs(self, dirs: Iterable[str | Path]) -> None: """Update configuration object from directories. This will first search for all YAML files within the given directories - and merge them together using :func:`dask.config.collect` (if identical values - are provided in multiple files, the value from the last file will be used). - Then, the - current configuration is merged with these new configuration options - using :func:`dask.config.merge` (new values are preferred over old - values). Nested objects are properly considered; see - :func:`dask.config.update` for details. + and merge them together using :func:`dask.config.collect` (if identical + values are provided in multiple files, the value from the last file + will be used). Then, the current configuration is merged with these + new configuration options using :func:`dask.config.merge` (new values + are preferred over old values). Nested objects are properly considered; + see :func:`dask.config.update` for details. Note ----