diff --git a/src/sirocco/core/workflow.py b/src/sirocco/core/workflow.py index 48b5d24f..1ae6b208 100644 --- a/src/sirocco/core/workflow.py +++ b/src/sirocco/core/workflow.py @@ -25,20 +25,19 @@ class Workflow: """Internal representation of a workflow""" - def __init__(self, - name: str, - rootdir: Path, - cycles: list[ConfigCycle], - tasks: list[ConfigTask], - data: ConfigData, - parameters: dict[str, list], - ) -> None: - + def __init__( + self, + name: str, + rootdir: Path, + cycles: list[ConfigCycle], + tasks: list[ConfigTask], + data: ConfigData, + parameters: dict[str, list], + ) -> None: self.name: str = name if not isinstance(rootdir, Path): msg = f"Need to provide path for rootdir but got {type(rootdir)}" raise TypeError(msg) - # TODO rename to rootdir since it does not make sense with this constructor self.config_rootdir: Path = rootdir self.tasks: Store = Store() @@ -104,6 +103,7 @@ def iter_coordinates(param_refs: list, date: datetime | None = None) -> Iterator @staticmethod def _build_data_dict(data: ConfigData) -> dict[str, ConfigAvailableData | ConfigGeneratedData]: return {data.name: data for data in chain(data.available, data.generated)} + @staticmethod def _build_tasks_dict(tasks: list[ConfigTask]) -> dict[str, ConfigTask]: return {task.name: task for task in tasks} @@ -115,7 +115,6 @@ def cycle_dates(cycle_config: ConfigCycle) -> Iterator[datetime]: while (date := date + cycle_config.period) < cycle_config.end_date: yield date - # TODO to be more consistent with `ConfigWorkflow` I would rename `from_yaml` to `from_config_file`, in a separate PR/commit, what you think? @classmethod def from_yaml(cls: type[Self], config_filename: str) -> Self: """ @@ -125,7 +124,6 @@ def from_yaml(cls: type[Self], config_filename: str) -> Self: """ return cls.from_config_workflow(ConfigWorkflow.from_config_file(config_filename)) - @classmethod def from_config_workflow(cls: type[Self], config_workflow: ConfigWorkflow) -> Workflow: return cls( diff --git a/src/sirocco/parsing/_yaml_data_models.py b/src/sirocco/parsing/_yaml_data_models.py index 3afffd00..92013913 100644 --- a/src/sirocco/parsing/_yaml_data_models.py +++ b/src/sirocco/parsing/_yaml_data_models.py @@ -23,15 +23,16 @@ from sirocco.parsing._utils import TimeUtils -# TODO if the type is not specified why bother specifying it? Isn't list[ITEM_T] similar useful for type checking as list at the moment? ITEM_T = typing.TypeVar("ITEM_T") + def list_not_empty(value: list[ITEM_T]) -> list[ITEM_T]: if len(value) < 1: msg = "At least one element is required." raise ValueError(msg) return value + class _NamedBaseModel(BaseModel): """Base class for all classes with a key that specifies their name. @@ -514,6 +515,7 @@ def get_plugin_from_named_base_model( Discriminator(get_plugin_from_named_base_model), ] + class ConfigWorkflow(BaseModel): """ The root of the configuration tree. diff --git a/tests/test_wc_workflow.py b/tests/test_wc_workflow.py index 7d824e78..b00162fa 100644 --- a/tests/test_wc_workflow.py +++ b/tests/test_wc_workflow.py @@ -50,7 +50,6 @@ def config_paths(request): def test_parse_config_file(config_paths, pprinter): reference_str = config_paths["txt"].read_text() - # TODO to be more consistent with `ConfigWorkflow` I would rename `from_yaml` to `from_config_file`, in a separate PR/commit, what you think? test_str = pprinter.format(Workflow.from_yaml(config_paths["yml"])) if test_str != reference_str: new_path = Path(config_paths["txt"]).with_suffix(".new.txt")