From 673a0690e5f2e101866a3c19dc7cf05600337b69 Mon Sep 17 00:00:00 2001 From: SarahAlidoost <55081872+SarahAlidoost@users.noreply.github.com> Date: Thu, 8 Jul 2021 16:15:06 +0200 Subject: [PATCH] Make path consistent (#143) --- docs/conf.py | 25 ++++++- ewatercycle/config/_config_object.py | 11 ++- ewatercycle/forcing/__init__.py | 13 ++-- ewatercycle/forcing/_default.py | 31 +++++++-- ewatercycle/models/abstract.py | 15 +++++ ewatercycle/models/lisflood.py | 12 ++-- ewatercycle/models/marrmot.py | 16 ++--- ewatercycle/models/pcrglobwb.py | 20 ++---- ewatercycle/models/wflow.py | 13 ++-- ewatercycle/observation/grdc.py | 7 +- ewatercycle/parameter_sets/default.py | 18 ++--- ewatercycle/util.py | 25 +++++++ tests/forcing/test_default.py | 67 +++++++++++++++++-- tests/models/test_marrmotm01.py | 21 ++++++ tests/models/test_wflow.py | 25 ++++++- .../test_default_parameterset.py | 18 ++--- tests/test_util.py | 53 ++++++++++++++- 17 files changed, 304 insertions(+), 86 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index b39af4c7..46cb64b3 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -32,7 +32,7 @@ # Add any Sphinx extension module names here, as strings. They can be # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom # ones. -extensions = ['sphinx.ext.autodoc', 'sphinx.ext.napoleon', 'nbsphinx'] +extensions = ['sphinx.ext.autodoc', 'sphinx.ext.napoleon', 'nbsphinx', 'sphinx.ext.intersphinx'] # Add any paths that contain templates here, relative to this directory. templates_path = ['_templates'] @@ -56,9 +56,9 @@ # built documents. # # The short X.Y version. -from ewatercycle.version import __version__ as version # The full version, including alpha/beta/rc tags. -release = version +# TODO update version +release = '1.0.0' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. @@ -240,3 +240,22 @@ def setup(app): ('pcrglobwb', 'params_style'), ('wflow', 'params_style'), ] + +intersphinx_mapping = { + 'cf_units': ('https://scitools.org.uk/cf-units/docs/latest/', None), + 'esmvalcore': + (f'https://docs.esmvaltool.org/projects/esmvalcore/en/latest/', + None), + 'esmvaltool': (f'https://docs.esmvaltool.org/en/latest/', None), + 'grpc4bmi': (f'https://grpc4bmi.readthedocs.io/en/latest/', None), + 'iris': ('https://scitools-iris.readthedocs.io/en/latest/', None), + 'lime': ('https://lime-ml.readthedocs.io/en/latest/', None), + 'basic_modeling_interface': ('https://bmi.readthedocs.io/en/latest/', None), + 'matplotlib': ('https://matplotlib.org/', None), + 'numpy': ('https://numpy.org/doc/stable/', None), + 'pandas': ('https://pandas.pydata.org/pandas-docs/dev', None), + 'python': ('https://docs.python.org/3/', None), + 'scipy': ('https://docs.scipy.org/doc/scipy/reference/', None), + 'seaborn': ('https://seaborn.pydata.org/', None), + 'sklearn': ('https://scikit-learn.org/stable', None), +} diff --git a/ewatercycle/config/_config_object.py b/ewatercycle/config/_config_object.py index 098a3d46..b1c1b09c 100644 --- a/ewatercycle/config/_config_object.py +++ b/ewatercycle/config/_config_object.py @@ -11,6 +11,8 @@ from ._validators import _validators from ._validated_config import ValidatedConfig +from ewatercycle.util import to_absolute_path + logger = getLogger(__name__) @@ -35,7 +37,6 @@ def _load_user_config(cls, filename: Union[os.PathLike, str]) -> 'Config': Name of the config file, must be yaml format """ new = cls() - mapping = read_config_file(filename) mapping['ewatercycle_config'] = filename @@ -45,11 +46,9 @@ def _load_user_config(cls, filename: Union[os.PathLike, str]) -> 'Config': return new @classmethod - def _load_default_config(cls, filename: Union[os.PathLike, - str]) -> 'Config': + def _load_default_config(cls, filename: Union[os.PathLike, str]) -> 'Config': """Load the default configuration.""" new = cls() - mapping = read_config_file(filename) new.update(mapping) @@ -57,7 +56,7 @@ def _load_default_config(cls, filename: Union[os.PathLike, def load_from_file(self, filename: Union[os.PathLike, str]) -> None: """Load user configuration from the given file.""" - path = Path(filename).expanduser() + path = to_absolute_path(str(filename)) if not path.exists(): raise FileNotFoundError(f'Cannot find: `{filename}') @@ -119,7 +118,7 @@ def save_to_file(self, config_file: Optional[Union[os.PathLike, str]] = None): def read_config_file(config_file: Union[os.PathLike, str]) -> dict: """Read config user file and store settings in a dictionary.""" - config_file = Path(config_file) + config_file = to_absolute_path(str(config_file)) if not config_file.exists(): raise IOError(f'Config file `{config_file}` does not exist.') diff --git a/ewatercycle/forcing/__init__.py b/ewatercycle/forcing/__init__.py index 7f276285..84d51ea9 100644 --- a/ewatercycle/forcing/__init__.py +++ b/ewatercycle/forcing/__init__.py @@ -6,6 +6,8 @@ from ._default import DefaultForcing, FORCING_YAML from . import _hype, _lisflood, _marrmot, _pcrglobwb, _wflow +from ewatercycle.util import to_absolute_path + FORCING_CLASSES: Dict[str, Type[DefaultForcing]] = { "hype": _hype.HypeForcing, "lisflood": _lisflood.LisfloodForcing, @@ -25,17 +27,18 @@ def load(directory: str) -> DefaultForcing: Returns: Forcing object """ yaml = YAML() - source = Path(directory) / FORCING_YAML + source = to_absolute_path(directory) # TODO give nicer error yaml.register_class(DefaultForcing) for forcing_cls in FORCING_CLASSES.values(): yaml.register_class(forcing_cls) - content = source.read_text() # Set directory in yaml string to parent of yaml file # Because in DefaultForcing.save the directory was removed - abs_dir = str(source.parent.expanduser().resolve()) - content += f'directory: {abs_dir}\n' - forcing_info = yaml.load(content) + forcing_info = yaml.load(source / FORCING_YAML) + forcing_info.directory = source + if forcing_info.shape: + forcing_info.shape = to_absolute_path(forcing_info.shape, parent = source) + return forcing_info diff --git a/ewatercycle/forcing/_default.py b/ewatercycle/forcing/_default.py index 3cbecbd3..e06b2199 100644 --- a/ewatercycle/forcing/_default.py +++ b/ewatercycle/forcing/_default.py @@ -2,9 +2,14 @@ from copy import copy from pathlib import Path from typing import Optional +import logging from ruamel.yaml import YAML +from ewatercycle.util import to_absolute_path + +logger = logging.getLogger(__name__) + FORCING_YAML = 'ewatercycle_forcing.yaml' @@ -26,8 +31,18 @@ def __init__(self, shape: Optional[str] = None): self.start_time = start_time self.end_time = end_time - self.directory = directory - self.shape = shape + self.directory = to_absolute_path(directory) + self.shape = to_absolute_path(shape) if shape is not None else shape + + def __str__(self): + """Nice formatting of forcing object.""" + return "\n".join( + [ + "eWaterCycle forcing", + "-------------------", + ] + + [f"{k}={v!s}" for k, v in self.__dict__.items()] + ) @classmethod def generate( @@ -45,11 +60,19 @@ def save(self): """Export forcing data for later use.""" yaml = YAML() yaml.register_class(self.__class__) - target = Path(self.directory) / FORCING_YAML + target = self.directory / FORCING_YAML # We want to make the yaml and its parent movable, - # so the directory should not be included in the yaml file + # so the directory and shape should not be included in the yaml file clone = copy(self) del clone.directory + + if clone.shape: + try: + clone.shape = str(clone.shape.relative_to(self.directory)) + except ValueError: + clone.shape = None + logger.info(f"Shapefile {self.shape} is not in forcing directory {self.directory}. So, it won't be saved in {target}.") + with open(target, 'w') as f: yaml.dump(clone, f) return target diff --git a/ewatercycle/models/abstract.py b/ewatercycle/models/abstract.py index 85592110..8e741b52 100644 --- a/ewatercycle/models/abstract.py +++ b/ewatercycle/models/abstract.py @@ -1,4 +1,5 @@ import logging +import textwrap from abc import ABCMeta, abstractmethod from datetime import datetime from typing import Tuple, Iterable, Any, TypeVar, Generic, Optional, ClassVar, Set @@ -42,6 +43,20 @@ def __del__(self): except AttributeError: pass + def __str__(self): + """Nice formatting of model object.""" + return "\n".join( + [ + f"eWaterCycle {self.__class__.__name__}", + "-------------------", + f"Version = {self.version}", + "Parameter set = ", + textwrap.indent(str(self.parameter_set), ' '), + "Forcing = ", + textwrap.indent(str(self.forcing), ' '), + ] + ) + @abstractmethod def setup(self, *args, **kwargs) -> Tuple[str, str]: """Performs model setup. diff --git a/ewatercycle/models/lisflood.py b/ewatercycle/models/lisflood.py index 5cf42686..12872b27 100644 --- a/ewatercycle/models/lisflood.py +++ b/ewatercycle/models/lisflood.py @@ -14,7 +14,7 @@ from ewatercycle.models.abstract import AbstractModel from ewatercycle.parameter_sets import ParameterSet from ewatercycle.parametersetdb.config import AbstractConfig -from ewatercycle.util import get_time, find_closest_point +from ewatercycle.util import get_time, find_closest_point, to_absolute_path class Lisflood(AbstractModel[LisfloodForcing]): @@ -84,7 +84,7 @@ def setup(self, # type: ignore """ # TODO forcing can be a part of parameter_set - cfg_dir_as_path = Path(cfg_dir) if cfg_dir else None + cfg_dir_as_path = to_absolute_path(cfg_dir) if cfg_dir else None cfg_dir_as_path = _generate_workdir(cfg_dir_as_path) config_file = self._create_lisflood_config(cfg_dir_as_path, start_time, end_time, IrrigationEfficiency, MaskMap) @@ -94,7 +94,7 @@ def setup(self, # type: ignore str(self.forcing_dir) ] if MaskMap is not None: - mask_map = Path(MaskMap).expanduser().resolve() + mask_map = to_absolute_path(MaskMap) try: mask_map.relative_to(self.parameter_set.directory) except ValueError: @@ -128,7 +128,7 @@ def _check_forcing(self, forcing): # if not warn users to run reindex_forcings if isinstance(forcing, LisfloodForcing): self.forcing = forcing - self.forcing_dir = Path(forcing.directory).expanduser().resolve() + self.forcing_dir = to_absolute_path(forcing.directory) # convert date_strings to datetime objects self._start = get_time(forcing.start_time) self._end = get_time(forcing.end_time) @@ -168,7 +168,7 @@ def _create_lisflood_config(self, cfg_dir: Path, start_time_iso: str = None, end if IrrigationEfficiency is not None: settings['IrrigationEfficiency'] = IrrigationEfficiency if MaskMap is not None: - mask_map = Path(MaskMap).expanduser().resolve() + mask_map = to_absolute_path(MaskMap) settings['MaskMap'] = str(mask_map.with_suffix('')) for textvar in self.cfg.config.iter("textvar"): @@ -308,7 +308,7 @@ def _generate_workdir(cfg_dir: Path = None) -> Path: scratch_dir = CFG['output_dir'] # TODO this timestamp isnot safe for parallel processing timestamp = datetime.datetime.now(datetime.timezone.utc).strftime("%Y%m%d_%H%M%S") - cfg_dir = Path(scratch_dir) / f'lisflood_{timestamp}' + cfg_dir = to_absolute_path(f'lisflood_{timestamp}', parent=Path(scratch_dir)) cfg_dir.mkdir(parents=True, exist_ok=True) return cfg_dir diff --git a/ewatercycle/models/marrmot.py b/ewatercycle/models/marrmot.py index 5831ebaf..dcce15c8 100644 --- a/ewatercycle/models/marrmot.py +++ b/ewatercycle/models/marrmot.py @@ -14,7 +14,7 @@ from ewatercycle import CFG from ewatercycle.forcing._marrmot import MarrmotForcing from ewatercycle.models.abstract import AbstractModel -from ewatercycle.util import get_time +from ewatercycle.util import get_time, to_absolute_path logger = logging.getLogger(__name__) @@ -38,7 +38,7 @@ def _generate_cfg_dir(cfg_dir: Path = None) -> Path: scratch_dir = CFG['output_dir'] # TODO this timestamp isnot safe for parallel processing timestamp = datetime.datetime.now(datetime.timezone.utc).strftime("%Y%m%d_%H%M%S") - cfg_dir = Path(scratch_dir) / f'marrmot_{timestamp}' + cfg_dir = to_absolute_path(f'marrmot_{timestamp}', parent=Path(scratch_dir)) cfg_dir.mkdir(parents=True, exist_ok=True) return cfg_dir @@ -63,7 +63,7 @@ class MarrmotM01(AbstractModel[MarrmotForcing]): def __init__(self, version: str, forcing: MarrmotForcing): """Construct MarrmotM01 with initial values. """ - super().__init__(version) + super().__init__(version, forcing=forcing) self._parameters = [1000.0] self.store_ini = [900.0] self.solver = Solver() @@ -114,7 +114,7 @@ def setup(self, # type: ignore self.store_ini = [initial_soil_moisture_storage] if solver: self.solver = solver - cfg_dir_as_path = Path(cfg_dir) if cfg_dir else None + cfg_dir_as_path = to_absolute_path(cfg_dir) if cfg_dir else None cfg_dir_as_path = _generate_cfg_dir(cfg_dir_as_path) config_file = self._create_marrmot_config(cfg_dir_as_path, start_time, end_time) @@ -141,7 +141,7 @@ def setup(self, # type: ignore def _check_forcing(self, forcing): """"Check forcing argument and get path, start and end time of forcing data.""" if isinstance(forcing, MarrmotForcing): - forcing_dir = Path(forcing.directory).expanduser().resolve() + forcing_dir = to_absolute_path(forcing.directory) self.forcing_file = str(forcing_dir / forcing.forcing_file) # convert date_strings to datetime objects self.forcing_start_time = get_time(forcing.start_time) @@ -288,7 +288,7 @@ class MarrmotM14(AbstractModel[MarrmotForcing]): def __init__(self, version: str, forcing: MarrmotForcing): """Construct MarrmotM14 with initial values. """ - super().__init__(version) + super().__init__(version, forcing=forcing) self._parameters = [1000.0, 0.5, 0.5, 100.0, 0.5, 4.25, 2.5] self.store_ini = [900.0, 900.0] self.solver = Solver() @@ -358,7 +358,7 @@ def setup(self, # type: ignore self.store_ini[1] = initial_saturated_zone_storage if solver: self.solver = solver - cfg_dir_as_path = Path(cfg_dir) if cfg_dir else None + cfg_dir_as_path = to_absolute_path(cfg_dir) if cfg_dir else None cfg_dir_as_path = _generate_cfg_dir(cfg_dir_as_path) config_file = self._create_marrmot_config(cfg_dir_as_path, start_time, end_time) @@ -385,7 +385,7 @@ def setup(self, # type: ignore def _check_forcing(self, forcing): """"Check forcing argument and get path, start and end time of forcing data.""" if isinstance(forcing, MarrmotForcing): - forcing_dir = Path(forcing.directory).expanduser().resolve() + forcing_dir = to_absolute_path(forcing.directory) self.forcing_file = str(forcing_dir / forcing.forcing_file) # convert date_strings to datetime objects self.forcing_start_time = get_time(forcing.start_time) diff --git a/ewatercycle/models/pcrglobwb.py b/ewatercycle/models/pcrglobwb.py index 0357fa1e..7d0336da 100644 --- a/ewatercycle/models/pcrglobwb.py +++ b/ewatercycle/models/pcrglobwb.py @@ -1,6 +1,5 @@ import datetime from os import PathLike -from pathlib import Path from typing import Any, Iterable, Tuple import numpy as np @@ -15,7 +14,7 @@ from ewatercycle.models.abstract import AbstractModel from ewatercycle.parameter_sets import ParameterSet from ewatercycle.parametersetdb.config import CaseConfigParser -from ewatercycle.util import get_time +from ewatercycle.util import get_time, to_absolute_path class PCRGlobWB(AbstractModel[PCRGlobWBForcing]): @@ -51,12 +50,11 @@ def _set_docker_image(self): def _setup_work_dir(self, cfg_dir: str = None): if cfg_dir: - work_dir = Path(cfg_dir) + self.work_dir = to_absolute_path(cfg_dir) else: # Must exist before setting up default config timestamp = datetime.datetime.now(datetime.timezone.utc).strftime("%Y%m%d_%H%M%S") - work_dir = Path(CFG["output_dir"]) / f"pcrglobwb_{timestamp}" - self.work_dir = work_dir.expanduser().resolve() + self.work_dir = to_absolute_path(f"pcrglobwb_{timestamp}", parent=CFG["output_dir"]) self.work_dir.mkdir(parents=True, exist_ok=True) def _setup_default_config(self): @@ -80,18 +78,14 @@ def _setup_default_config(self): "meteoOptions", "temperatureNC", str( - (Path(self.forcing.directory) / self.forcing.temperatureNC) - .expanduser() - .resolve() + to_absolute_path(self.forcing.temperatureNC, parent=self.forcing.directory) ), ) cfg.set( "meteoOptions", "precipitationNC", str( - (Path(self.forcing.directory) / self.forcing.precipitationNC) - .expanduser() - .resolve() + to_absolute_path(self.forcing.precipitationNC, parent=self.forcing.directory) ), ) @@ -167,11 +161,11 @@ def _update_config(self, **kwargs): def _export_config(self) -> PathLike: self.config.set("globalOptions", "outputDir", str(self.work_dir)) - new_cfg_file = Path(self.work_dir) / "pcrglobwb_ewatercycle.ini" + new_cfg_file = to_absolute_path("pcrglobwb_ewatercycle.ini", parent=self.work_dir) with new_cfg_file.open("w") as filename: self.config.write(filename) - self.cfg_file = new_cfg_file.expanduser().resolve() + self.cfg_file = new_cfg_file return self.cfg_file def _start_container(self): diff --git a/ewatercycle/models/wflow.py b/ewatercycle/models/wflow.py index 360a4eea..53a492ec 100644 --- a/ewatercycle/models/wflow.py +++ b/ewatercycle/models/wflow.py @@ -15,7 +15,7 @@ from ewatercycle.models.abstract import AbstractModel from ewatercycle.parameter_sets import ParameterSet from ewatercycle.parametersetdb.config import CaseConfigParser -from ewatercycle.util import get_time +from ewatercycle.util import get_time, to_absolute_path class Wflow(AbstractModel[WflowForcing]): @@ -85,24 +85,23 @@ def setup(self, cfg_dir: str = None, **kwargs) -> Tuple[str, str]: # type: igno if "end_time" in kwargs: cfg.set("run", "endtime", _iso_to_wflow(kwargs["end_time"])) - updated_cfg_file = self.work_dir / "wflow_ewatercycle.ini" + updated_cfg_file = to_absolute_path("wflow_ewatercycle.ini", parent= self.work_dir) with updated_cfg_file.open("w") as filename: cfg.write(filename) self._start_container() return ( - str(updated_cfg_file.expanduser().resolve()), + str(updated_cfg_file), str(self.work_dir), ) def _setup_working_directory(self, cfg_dir: str = None): if cfg_dir: - working_directory = Path(cfg_dir) + self.work_dir = to_absolute_path(cfg_dir) else: timestamp = datetime.datetime.now(datetime.timezone.utc).strftime("%Y%m%d_%H%M%S") - working_directory = CFG["output_dir"] / f"wflow_{timestamp}" - self.work_dir = working_directory.expanduser().resolve() + self.work_dir = to_absolute_path(f"wflow_{timestamp}", parent=CFG["output_dir"]) # Make sure parents exist self.work_dir.parent.mkdir(parents=True, exist_ok=True) @@ -110,7 +109,7 @@ def _setup_working_directory(self, cfg_dir: str = None): shutil.copytree(src=self.parameter_set.directory, dst=self.work_dir) if self.forcing: - forcing_path = Path(self.forcing.directory) / self.forcing.netcdfinput + forcing_path = to_absolute_path(self.forcing.netcdfinput, parent= self.forcing.directory) shutil.copy(src=forcing_path, dst=self.work_dir) def _start_container(self): diff --git a/ewatercycle/observation/grdc.py b/ewatercycle/observation/grdc.py index 80012d78..70893a19 100644 --- a/ewatercycle/observation/grdc.py +++ b/ewatercycle/observation/grdc.py @@ -1,11 +1,10 @@ import os -from pathlib import Path from typing import Dict, Tuple, Union import logging import pandas as pd from ewatercycle import CFG -from ewatercycle.util import get_time +from ewatercycle.util import get_time, to_absolute_path logger = logging.getLogger(__name__) @@ -76,10 +75,10 @@ def get_grdc_data(station_id: str, 'nrMissingData': 0} """ if data_home: - data_path = Path(data_home).expanduser().resolve() + data_path = to_absolute_path(data_home) else: if CFG["grdc_location"]: - data_path = Path(CFG["grdc_location"]).expanduser().resolve() + data_path = to_absolute_path(CFG["grdc_location"]) else: raise ValueError( f'Provide the grdc path using `data_home` argument ' diff --git a/ewatercycle/parameter_sets/default.py b/ewatercycle/parameter_sets/default.py index dd7022c3..eae3c5b4 100644 --- a/ewatercycle/parameter_sets/default.py +++ b/ewatercycle/parameter_sets/default.py @@ -2,6 +2,7 @@ from typing import Set, Optional from ewatercycle import CFG +from ewatercycle.util import to_absolute_path class ParameterSet: @@ -29,14 +30,14 @@ def __init__( supported_model_versions: Optional[Set[str]] = None, ): self.name = name - self.directory = _make_absolute(directory) - self.config = _make_absolute(config) + self.directory = to_absolute_path(directory, parent = CFG.get("parameterset_dir")) + self.config = to_absolute_path(config, parent = CFG.get("parameterset_dir")) self.doi = doi self.target_model = target_model self.supported_model_versions = set() if supported_model_versions is None else supported_model_versions def __repr__(self): - options = ", ".join(f"{k}={v!r}" for k, v in self.__dict__.items()) + options = ", ".join(f"{k}={v!s}" for k, v in self.__dict__.items()) return f"ParameterSet({options})" def __str__(self): @@ -46,7 +47,7 @@ def __str__(self): "Parameter set", "-------------", ] - + [f"{k}={v!r}" for k, v in self.__dict__.items()] + + [f"{k}={v!s}" for k, v in self.__dict__.items()] ) @property @@ -54,12 +55,3 @@ def is_available(self) -> bool: """Tests if directory and config file is available on this machine""" return self.directory.exists() and self.config.exists() - -def _make_absolute(input_path: str) -> Path: - pathlike = Path(input_path) - if pathlike.is_absolute(): - return pathlike - if CFG["parameterset_dir"]: - return CFG["parameterset_dir"] / pathlike - else: - raise ValueError(f'CFG["parameterset_dir"] is not set. Unable to make {input_path} relative to it') diff --git a/ewatercycle/util.py b/ewatercycle/util.py index 37904138..e7acd6d9 100644 --- a/ewatercycle/util.py +++ b/ewatercycle/util.py @@ -1,4 +1,6 @@ +from os import PathLike from typing import Any, Iterable, Tuple, Dict +from pathlib import Path import fiona import numpy as np @@ -169,3 +171,26 @@ def data_files_from_recipe_output(recipe_output: RecipeOutput) -> Tuple[str, Dic # TODO simplify (recipe_output.location) when next esmvalcore release is made directory = str(data_files[0].filename.parent) return directory, forcing_files + + +def to_absolute_path(input_path: str, parent: Path = None, must_exist: bool = False) -> Path: + """Parse input string as :py:class:`pathlib.Path` object. + + Args: + input_path: Input string path that can be a relative or absolute path. + parent: Optional parent path of the input path + must_exist: Optional argument to check if the input path exists. + + Returns: + The input path that is an absolute path and a :py:class:`pathlib.Path` object. + """ + pathlike = Path(input_path) + if parent: + pathlike = parent.joinpath(pathlike) + try: + pathlike.relative_to(parent) + except ValueError: + raise ValueError(f"Input path {input_path} is not a subpath of parent {parent}") + + return pathlike.expanduser().resolve(strict=must_exist) + diff --git a/tests/forcing/test_default.py b/tests/forcing/test_default.py index 93b85e12..a635974a 100644 --- a/tests/forcing/test_default.py +++ b/tests/forcing/test_default.py @@ -1,3 +1,4 @@ +import logging import pytest from ewatercycle.forcing import generate, load_foreign, DefaultForcing, load, FORCING_YAML @@ -26,30 +27,73 @@ def test_load_foreign_unknown(): @pytest.fixture -def sample_forcing_yaml_content(sample_shape, tmp_path): +def sample_forcing_yaml_content(): return ''.join([ '!DefaultForcing\n', "start_time: '1989-01-02T00:00:00Z'\n", "end_time: '1999-01-02T00:00:00Z'\n", - f'shape: {sample_shape}\n' + "shape:\n" ]) +@pytest.fixture +def sample_forcing_yaml_content_with_shape(): + return ''.join([ + '!DefaultForcing\n', + "start_time: '1989-01-02T00:00:00Z'\n", + "end_time: '1999-01-02T00:00:00Z'\n", + "shape: myshape.shp\n" + ]) -def test_save(sample_shape, tmp_path, sample_forcing_yaml_content): +def test_save_with_shapefile_outside_forcing_dir(sample_shape, tmp_path, sample_forcing_yaml_content, caplog): forcing = DefaultForcing( directory=str(tmp_path), start_time='1989-01-02T00:00:00Z', end_time='1999-01-02T00:00:00Z', shape=sample_shape ) + with caplog.at_level(logging.INFO): + forcing.save() + + file = tmp_path / FORCING_YAML + written = file.read_text() + expected = sample_forcing_yaml_content + assert written == expected + assert 'is not in forcing directory' in caplog.text + +def test_save_with_shapefile_inside_forcing_dir(tmp_path, sample_forcing_yaml_content_with_shape, caplog): + + forcing = DefaultForcing( + directory=str(tmp_path), + start_time='1989-01-02T00:00:00Z', + end_time='1999-01-02T00:00:00Z', + shape=str(tmp_path / "myshape.shp") + ) + with caplog.at_level(logging.INFO): + forcing.save() + + file = tmp_path / FORCING_YAML + written = file.read_text() + expected = sample_forcing_yaml_content_with_shape + assert written == expected + assert 'is not in forcing directory' not in caplog.text + + +def test_save_without_shapefile(tmp_path, sample_forcing_yaml_content): + + forcing = DefaultForcing( + directory=str(tmp_path), + start_time='1989-01-02T00:00:00Z', + end_time='1999-01-02T00:00:00Z', + ) forcing.save() + file = tmp_path / FORCING_YAML written = file.read_text() expected = sample_forcing_yaml_content assert written == expected -def test_load(sample_shape, tmp_path, sample_forcing_yaml_content): +def test_load(tmp_path, sample_forcing_yaml_content): file = tmp_path / FORCING_YAML file.write_text(sample_forcing_yaml_content) result = load(tmp_path) @@ -57,7 +101,20 @@ def test_load(sample_shape, tmp_path, sample_forcing_yaml_content): directory=str(tmp_path), start_time='1989-01-02T00:00:00Z', end_time='1999-01-02T00:00:00Z', - shape=sample_shape + ) + assert result == expected + + +def test_load_with_shape(tmp_path, sample_forcing_yaml_content_with_shape): + file = tmp_path / FORCING_YAML + file.write_text(sample_forcing_yaml_content_with_shape) + result = load(tmp_path) + expected = DefaultForcing( + directory=str(tmp_path), + start_time='1989-01-02T00:00:00Z', + end_time='1999-01-02T00:00:00Z', + shape=tmp_path / "myshape.shp" + ) assert result == expected diff --git a/tests/models/test_marrmotm01.py b/tests/models/test_marrmotm01.py index 989083f1..a40099ba 100644 --- a/tests/models/test_marrmotm01.py +++ b/tests/models/test_marrmotm01.py @@ -51,6 +51,27 @@ def model_with_setup(self, model: MarrmotM01): cfg_file, cfg_dir = model.setup() return model, cfg_file, cfg_dir + + def test_str(self, model, forcing_file): + actual = str(model) + expected = "\n".join( + [ + "eWaterCycle MarrmotM01", + "-------------------", + "Version = 2020.11", + "Parameter set = ", + " None", + "Forcing = ", + " eWaterCycle forcing", + " -------------------", + " start_time=1989-01-01T00:00:00Z", + " end_time=1992-12-31T00:00:00Z", + f" directory={str(Path(forcing_file).parent)}", + " shape=None", + " forcing_file=BMI_testcase_m01_BuffaloRiver_TN_USA.mat", + ]) + assert actual == expected + def test_parameters(self, model): expected = [ diff --git a/tests/models/test_wflow.py b/tests/models/test_wflow.py index b083d656..ac3ccbc2 100644 --- a/tests/models/test_wflow.py +++ b/tests/models/test_wflow.py @@ -15,7 +15,7 @@ def mocked_config(tmp_path): CFG['output_dir'] = tmp_path CFG['container_engine'] = 'singularity' CFG['singularity_dir'] = tmp_path - CFG['parameterset_dir'] = tmp_path / 'psr' + CFG['parameterset_dir'] = tmp_path / 'wflow_testcase' CFG['parameter_sets'] = {} @@ -40,12 +40,33 @@ def model(parameter_set): return Wflow(version="2020.1.1", parameter_set=parameter_set) +def test_str(model, tmp_path): + actual = str(model) + expected = "\n".join( + [ + "eWaterCycle Wflow", + "-------------------", + "Version = 2020.1.1", + "Parameter set = ", + " Parameter set", + " -------------", + " name=wflow_testcase", + f" directory={str(tmp_path / 'wflow_testcase')}", + f" config={str(tmp_path / 'wflow_testcase' / 'wflow_sbm_nc.ini')}", + " doi=N/A", + " target_model=wflow", + " supported_model_versions=set()", + "Forcing = ", + " None", + ]) + assert actual == expected + + def test_setup(model): with patch.object(BmiClientSingularity, '__init__', return_value=None), patch('datetime.datetime') as mocked_datetime: mocked_datetime.now.return_value = datetime(2021, 1, 2, 3, 4, 5) cfg_file, cfg_dir = model.setup() - expected_cfg_dir = CFG['output_dir'] / 'wflow_20210102_030405' assert cfg_dir == str(expected_cfg_dir) assert cfg_file == str(expected_cfg_dir / 'wflow_ewatercycle.ini') diff --git a/tests/parameter_sets/test_default_parameterset.py b/tests/parameter_sets/test_default_parameterset.py index 5cd3e3c8..d63306f4 100644 --- a/tests/parameter_sets/test_default_parameterset.py +++ b/tests/parameter_sets/test_default_parameterset.py @@ -37,10 +37,10 @@ def test_is_available(self, parameter_set: ParameterSet): def test_repr(self, parameter_set: ParameterSet, tmp_path): expected = ( - "ParameterSet(name='justatest', " - f"directory=PosixPath('{tmp_path}'), " - f"config=PosixPath('{tmp_path}/mymockedconfig.ini'), " - "doi='N/A', target_model='generic', supported_model_versions=set())" + "ParameterSet(name=justatest, " + f"directory={str(tmp_path)}, " + f"config={str(tmp_path)}/mymockedconfig.ini, " + "doi=N/A, target_model=generic, supported_model_versions=set())" ) assert repr(parameter_set) == expected @@ -48,11 +48,11 @@ def test_str(self, parameter_set: ParameterSet, tmp_path): expected = ( 'Parameter set\n' '-------------\n' - "name='justatest'\n" - f"directory=PosixPath('{tmp_path}')\n" - f"config=PosixPath('{tmp_path}/mymockedconfig.ini')\n" - "doi='N/A'\n" - "target_model='generic'\n" + "name=justatest\n" + f"directory={str(tmp_path)}\n" + f"config={str(tmp_path)}/mymockedconfig.ini\n" + "doi=N/A\n" + "target_model=generic\n" "supported_model_versions=set()" ) assert str(parameter_set) == expected diff --git a/tests/test_util.py b/tests/test_util.py index 85bd0f24..27e5fe71 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -1,8 +1,9 @@ from datetime import datetime, timezone +from pathlib import Path import pytest -from ewatercycle.util import get_time, find_closest_point +from ewatercycle.util import get_time, find_closest_point, to_absolute_path from numpy.testing import assert_array_almost_equal @@ -32,3 +33,53 @@ def test_find_closest_point(): actual_distances, actual_index = find_closest_point([-99.83, -99.32], [42.25, 42.21], -99.32, 43.25) assert_array_almost_equal(actual_distances, expected_distances) assert actual_index == expected_index + + +def test_to_absolute_path(): + input_path = "~/nonexistent_file.txt" + parsed = to_absolute_path(input_path) + expected = Path.home() / "nonexistent_file.txt" + assert parsed == expected + + +def test_to_absolute_path_must_exist(): + input_path = "~/nonexistent_file.txt" + with pytest.raises(FileNotFoundError): + to_absolute_path(input_path, must_exist=True) + + +def test_to_absolute_path_with_absolute_input_and_parent(tmp_path): + input_path = tmp_path / "nonexistent_file.txt" + parsed = to_absolute_path(str(input_path), parent = tmp_path) + assert parsed == input_path + + +def test_to_absolute_path_with_relative_input_and_parent(tmp_path): + input_path = "nonexistent_file.txt" + parsed = to_absolute_path(input_path, parent = tmp_path) + expected = tmp_path / "nonexistent_file.txt" + assert parsed == expected + + +def test_to_absolute_path_with_relative_input_and_no_parent(): + input_path = "nonexistent_file.txt" + parsed = to_absolute_path(input_path) + expected = Path.cwd() / "nonexistent_file.txt" + assert parsed == expected + + +def test_to_absolute_path_with_relative_input_and_relative_parent(): + input_path = "nonexistent_file.txt" + parsed = to_absolute_path(input_path, parent = Path('.')) + expected = Path.cwd() / "nonexistent_file.txt" + assert parsed == expected + + +def test_to_absolute_path_with_absolute_input_and_nonrelative_parent(tmp_path): + parent = tmp_path / 'parent_dir' + input_path = tmp_path / "nonexistent_file.txt" + + with pytest.raises(ValueError)as excinfo: + to_absolute_path(str(input_path), parent = parent) + + assert "is not a subpath of parent" in str(excinfo.value)