From 5e67c61a4c41f197bc3cf5ef52ccb9eb00abe6f9 Mon Sep 17 00:00:00 2001 From: gampnico <45390064+gampnico@users.noreply.github.com> Date: Thu, 28 Nov 2024 15:09:57 +0100 Subject: [PATCH 1/3] fix: relative paths to config files not found when using WRFxCSPY --- cosipy/config.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cosipy/config.py b/cosipy/config.py index 8166291..ce6ed50 100644 --- a/cosipy/config.py +++ b/cosipy/config.py @@ -4,6 +4,7 @@ import argparse import sys +import os from importlib.metadata import entry_points if sys.version_info >= (3, 11): @@ -18,12 +19,12 @@ def set_parser() -> argparse.ArgumentParser: "Coupled snowpack and ice surface energy and mass balance model in Python." ) parser = argparse.ArgumentParser(prog="COSIPY", description=tagline) - + current_directory = os.getcwd() # Optional arguments parser.add_argument( "-c", "--config", - default="./config.toml", + default=f"{current_directory}/config.toml", dest="config_path", type=str, metavar="", @@ -34,7 +35,7 @@ def set_parser() -> argparse.ArgumentParser: parser.add_argument( "-x", "--constants", - default="./constants.toml", + default=f"{current_directory}/constants.toml", dest="constants_path", type=str, metavar="", @@ -45,7 +46,7 @@ def set_parser() -> argparse.ArgumentParser: parser.add_argument( "-s", "--slurm", - default="./slurm_config.toml", + default=f"{current_directory}/slurm_config.toml", dest="slurm_path", type=str, metavar="", From c666f51963d0cec23a9b1f63df4eb7e30d2eb0f1 Mon Sep 17 00:00:00 2001 From: gampnico <45390064+gampnico@users.noreply.github.com> Date: Thu, 28 Nov 2024 16:03:26 +0100 Subject: [PATCH 2/3] fix: attempts pathing using environment variable --- cosipy/config.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cosipy/config.py b/cosipy/config.py index ce6ed50..6e1644f 100644 --- a/cosipy/config.py +++ b/cosipy/config.py @@ -19,12 +19,12 @@ def set_parser() -> argparse.ArgumentParser: "Coupled snowpack and ice surface energy and mass balance model in Python." ) parser = argparse.ArgumentParser(prog="COSIPY", description=tagline) - current_directory = os.getcwd() + directory_path = os.environ.get("COSIPY_DIR", ".") # Optional arguments parser.add_argument( "-c", "--config", - default=f"{current_directory}/config.toml", + default=f"{directory_path}/config.toml", dest="config_path", type=str, metavar="", @@ -35,7 +35,7 @@ def set_parser() -> argparse.ArgumentParser: parser.add_argument( "-x", "--constants", - default=f"{current_directory}/constants.toml", + default=f"{directory_path}/constants.toml", dest="constants_path", type=str, metavar="", @@ -46,7 +46,7 @@ def set_parser() -> argparse.ArgumentParser: parser.add_argument( "-s", "--slurm", - default=f"{current_directory}/slurm_config.toml", + default=f"{directory_path}/slurm_config.toml", dest="slurm_path", type=str, metavar="", From 59a17f0dffaec251a23af17efda8431739e6c158 Mon Sep 17 00:00:00 2001 From: gampnico <45390064+gampnico@users.noreply.github.com> Date: Fri, 29 Nov 2024 13:52:49 +0100 Subject: [PATCH 3/3] refactor(config): path type safety, default path handling for WRFxCSPY Refs: #81, #86, cryotools/wrf_x_cspy#1 --- cosipy/config.py | 47 ++++++++++++++++++++------- cosipy/tests/conftest.py | 9 ++++++ cosipy/tests/test_config.py | 55 ++++++++++++++++++++++++++++++++ cosipy/utilities/config_utils.py | 5 +-- 4 files changed, 103 insertions(+), 13 deletions(-) diff --git a/cosipy/config.py b/cosipy/config.py index 6e1644f..2d0d924 100644 --- a/cosipy/config.py +++ b/cosipy/config.py @@ -3,8 +3,9 @@ """ import argparse -import sys import os +import pathlib +import sys from importlib.metadata import entry_points if sys.version_info >= (3, 11): @@ -13,20 +14,44 @@ import tomli as tomllib # backwards compatibility +def get_cosipy_path_from_env(name: str = "COSIPY_DIR") -> pathlib.Path: + """Get path to COSIPY directory. + + When using WRFxCSPY, the coupler will default to searching for + config files in the current working directory, which may contain the + COSIPY source code. This function instead loads an environment + variable. + + Args: + name: Name of environment variable pointing to the COSIPY + directory. + + Returns: + Path to the COSIPY directory. + + Raises: + NotADirectoryError: Invalid path. + """ + cosipy_path = pathlib.Path(os.environ.get(name, os.getcwd())) + if not cosipy_path.is_dir(): + raise NotADirectoryError(f"Invalid path at: {cosipy_path}") + + return cosipy_path + + def set_parser() -> argparse.ArgumentParser: """Set argument parser for COSIPY.""" - tagline = ( - "Coupled snowpack and ice surface energy and mass balance model in Python." - ) + tagline = "Coupled snowpack and ice surface energy and mass balance model in Python." parser = argparse.ArgumentParser(prog="COSIPY", description=tagline) - directory_path = os.environ.get("COSIPY_DIR", ".") + cosipy_path = get_cosipy_path_from_env() + # Optional arguments parser.add_argument( "-c", "--config", - default=f"{directory_path}/config.toml", + default=cosipy_path / "config.toml", dest="config_path", - type=str, + type=pathlib.Path, metavar="", required=False, help="relative path to configuration file", @@ -35,9 +60,9 @@ def set_parser() -> argparse.ArgumentParser: parser.add_argument( "-x", "--constants", - default=f"{directory_path}/constants.toml", + default=cosipy_path / "constants.toml", dest="constants_path", - type=str, + type=pathlib.Path, metavar="", required=False, help="relative path to constants file", @@ -46,9 +71,9 @@ def set_parser() -> argparse.ArgumentParser: parser.add_argument( "-s", "--slurm", - default=f"{directory_path}/slurm_config.toml", + default=cosipy_path / "slurm_config.toml", dest="slurm_path", - type=str, + type=pathlib.Path, metavar="", required=False, help="relative path to Slurm configuration file", diff --git a/cosipy/tests/conftest.py b/cosipy/tests/conftest.py index 4539870..9a3dcf8 100644 --- a/cosipy/tests/conftest.py +++ b/cosipy/tests/conftest.py @@ -37,6 +37,15 @@ def conftest_mock_check_file_exists(): mock_exists.return_value = True +@pytest.fixture(scope="function", autouse=False) +def conftest_mock_check_directory_exists(): + """Override checks when mocking directories.""" + + patcher = patch("pathlib.Path.is_dir") + mock_exists = patcher.start() + mock_exists.return_value = True + + @pytest.fixture(scope="function", autouse=False) def conftest_disable_jit(): # numba.config.DISABLE_JIT = True diff --git a/cosipy/tests/test_config.py b/cosipy/tests/test_config.py index 4e6589f..adc00b2 100644 --- a/cosipy/tests/test_config.py +++ b/cosipy/tests/test_config.py @@ -1,4 +1,9 @@ import argparse +import os +import pathlib +from unittest.mock import patch + +import pytest import cosipy.config from cosipy.config import Config @@ -25,6 +30,56 @@ def test_set_parser(self): for name in ["help", "config", "constants", "slurm"]: assert name in actions + @pytest.mark.dependency(name="TestConfigParser::test_set_parser") + @pytest.mark.parametrize("arg_type", (str, pathlib.Path)) + def test_user_arguments(self, arg_type): + test_parser = cosipy.config.set_parser() + test_path = "./some/path/config.toml" + assert isinstance(test_path, str) + + test_args = [ + "--config", + test_path, + "--constants", + test_path, + "--slurm", + test_path, + ] + arguments, unknown = test_parser.parse_known_args(test_args) + assert isinstance(arguments, argparse.Namespace) + + for user_path in [ + arguments.config_path, + arguments.constants_path, + arguments.slurm_path, + ]: + assert isinstance(user_path, pathlib.Path) + assert user_path == pathlib.Path(test_path) + + @patch.dict(os.environ, {"COSIPY_DIR": "./path/to/wrong/cosipy/"}) + def test_check_directory_exists(self): + """Raise error if directory not found.""" + wrong_path = "./path/to/wrong/cosipy/" + error_message = f"Invalid path at: {pathlib.Path(wrong_path)}" + with pytest.raises(NotADirectoryError, match=error_message): + cosipy.config.get_cosipy_path_from_env(name="COSIPY_DIR") + + @pytest.mark.parametrize( + "arg_env", ((True, "COSIPY_DIR"), (True, "XFAIL"), (False, "")) + ) + @patch.dict(os.environ, {"COSIPY_DIR": "./path/to/cosipy/"}) + def test_get_cosipy_path( + self, arg_env, conftest_mock_check_directory_exists + ): + _ = conftest_mock_check_directory_exists + test_name = arg_env[1] + compare_path = cosipy.config.get_cosipy_path_from_env(name=test_name) + + if arg_env[1] == "COSIPY_DIR": + assert compare_path == pathlib.Path("./path/to/cosipy/") + else: + assert compare_path == pathlib.Path.cwd() + class TestConfig: """Test rtoml support.""" diff --git a/cosipy/utilities/config_utils.py b/cosipy/utilities/config_utils.py index 090c0b2..4327bb9 100644 --- a/cosipy/utilities/config_utils.py +++ b/cosipy/utilities/config_utils.py @@ -3,6 +3,7 @@ """ import argparse +import pathlib import sys from collections import namedtuple @@ -73,9 +74,9 @@ def set_arg_parser(cls) -> argparse.ArgumentParser: parser.add_argument( "-u", "--utilities", - default="./utilities_config.toml", + default=pathlib.Path("./utilities_config.toml"), dest="utilities_path", - type=str, + type=pathlib.Path, metavar="", required=False, help="relative path to utilities' configuration file",