From fa2c0ea4e807d0c6fbc5c5867d921ac51be620a4 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 27 Jan 2025 11:55:37 +0100 Subject: [PATCH 1/8] fix: make ICON namelist non optional --- src/sirocco/core/_tasks/icon_task.py | 2 +- src/sirocco/core/graph_items.py | 8 ++++---- src/sirocco/parsing/_yaml_data_models.py | 11 ++++++----- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/sirocco/core/_tasks/icon_task.py b/src/sirocco/core/_tasks/icon_task.py index 614dc451..cc5a2949 100644 --- a/src/sirocco/core/_tasks/icon_task.py +++ b/src/sirocco/core/_tasks/icon_task.py @@ -10,7 +10,7 @@ from sirocco.parsing._yaml_data_models import ConfigIconTaskSpecs -@dataclass +@dataclass(kw_only=True) class IconTask(ConfigIconTaskSpecs, Task): core_namelists: dict[str, f90nml.Namelist] = field(default_factory=dict) diff --git a/src/sirocco/core/graph_items.py b/src/sirocco/core/graph_items.py index 2453d2b3..d37df059 100644 --- a/src/sirocco/core/graph_items.py +++ b/src/sirocco/core/graph_items.py @@ -23,7 +23,7 @@ ) -@dataclass +@dataclass(kw_only=True) class GraphItem: """base class for Data Tasks and Cycles""" @@ -33,7 +33,7 @@ class GraphItem: coordinates: dict -@dataclass +@dataclass(kw_only=True) class Data(ConfigBaseDataSpecs, GraphItem): """Internal representation of a data node""" @@ -56,7 +56,7 @@ def from_config(cls, config: ConfigBaseData, coordinates: dict) -> Self: BoundData: TypeAlias = tuple[Data, str | None] -@dataclass +@dataclass(kw_only=True) class Task(ConfigBaseTaskSpecs, GraphItem): """Internal representation of a task node""" @@ -129,7 +129,7 @@ def link_wait_on_tasks(self, taskstore: Store): ) -@dataclass +@dataclass(kw_only=True) class Cycle(GraphItem): """Internal reprenstation of a cycle""" diff --git a/src/sirocco/parsing/_yaml_data_models.py b/src/sirocco/parsing/_yaml_data_models.py index 48568a23..60b419cd 100644 --- a/src/sirocco/parsing/_yaml_data_models.py +++ b/src/sirocco/parsing/_yaml_data_models.py @@ -276,7 +276,7 @@ def check_period_is_not_negative_or_zero(self) -> ConfigCycle: return self -@dataclass +@dataclass(kw_only=True) class ConfigBaseTaskSpecs: computer: str | None = None host: str | None = None @@ -349,7 +349,7 @@ def from_cli_argument(cls, arg: str) -> ShellCliArgument: return cls(name, references_data_item, cli_option_of_data_item) -@dataclass +@dataclass(kw_only=True) class ConfigShellTaskSpecs: plugin: ClassVar[Literal["shell"]] = "shell" command: str = "" @@ -409,7 +409,7 @@ def parse_cli_arguments(cli_arguments: str) -> list[ShellCliArgument]: return [ShellCliArgument.from_cli_argument(arg) for arg in ConfigShellTask.split_cli_arguments(cli_arguments)] -@dataclass +@dataclass(kw_only=True) class ConfigNamelist: """Class for namelist specifications""" @@ -417,10 +417,10 @@ class ConfigNamelist: specs: dict | None = None -@dataclass +@dataclass(kw_only=True) class ConfigIconTaskSpecs: plugin: ClassVar[Literal["icon"]] = "icon" - namelists: dict[str, ConfigNamelist] | None = None + namelists: dict[str, ConfigNamelist] class ConfigIconTask(ConfigBaseTask, ConfigIconTaskSpecs): @@ -463,6 +463,7 @@ class DataType(enum.StrEnum): @dataclass +@dataclass(kw_only=True) class ConfigBaseDataSpecs: type: DataType src: str From 2691656e7ebfbd15d7d20299252dba0c8d090287 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 27 Jan 2025 15:22:55 +0100 Subject: [PATCH 2/8] add:fix: ConfigNamelist doc + type --- src/sirocco/parsing/_yaml_data_models.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/sirocco/parsing/_yaml_data_models.py b/src/sirocco/parsing/_yaml_data_models.py index 60b419cd..9e603813 100644 --- a/src/sirocco/parsing/_yaml_data_models.py +++ b/src/sirocco/parsing/_yaml_data_models.py @@ -411,7 +411,21 @@ def parse_cli_arguments(cli_arguments: str) -> list[ShellCliArgument]: @dataclass(kw_only=True) class ConfigNamelist: - """Class for namelist specifications""" + """Class for namelist specifications + + - path is the path to the namelist file considered as template + - specs is a dictionnary containing the specifications of parameters + to change in the original namelist file + + For example: + + ... python + + ConfigNamelist(path="/some/path/to/icon.nml", + specs={"first_nml_block":{"first_param": first_value, + "second_param": second_value}, + "second_nml_block":{"third_param": third_value}}) + """ path: Path specs: dict | None = None @@ -424,12 +438,9 @@ class ConfigIconTaskSpecs: class ConfigIconTask(ConfigBaseTask, ConfigIconTaskSpecs): - # validation done here and not in ConfigNamelist so that we can still - # import ConfigIconTaskSpecs in core._tasks.IconTask. Hence the iteration - # over the namelists that could be avoided with a more raw pydantic design @field_validator("namelists", mode="before") @classmethod - def check_nml(cls, nml_list: list[Any]) -> ConfigNamelist: + def check_nml(cls, nml_list: list[Any]) -> dict[str, ConfigNamelist]: if nml_list is None: msg = "ICON tasks need namelists, got none" raise ValueError(msg) From e4045ef62183f36f7f64215d3a35320cd9dfda17 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 27 Jan 2025 15:57:18 +0100 Subject: [PATCH 3/8] doc: add doctest for ConfigNamelist also add the last 2 missing `kw_only=True` to @dataclass --- src/sirocco/core/_tasks/shell_task.py | 2 +- src/sirocco/parsing/_yaml_data_models.py | 13 ++++++------- src/sirocco/pretty_print.py | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/sirocco/core/_tasks/shell_task.py b/src/sirocco/core/_tasks/shell_task.py index d163e597..cc7dbdf8 100644 --- a/src/sirocco/core/_tasks/shell_task.py +++ b/src/sirocco/core/_tasks/shell_task.py @@ -6,6 +6,6 @@ from sirocco.parsing._yaml_data_models import ConfigShellTaskSpecs -@dataclass +@dataclass(kw_only=True) class ShellTask(ConfigShellTaskSpecs, Task): pass diff --git a/src/sirocco/parsing/_yaml_data_models.py b/src/sirocco/parsing/_yaml_data_models.py index 9e603813..37809aa1 100644 --- a/src/sirocco/parsing/_yaml_data_models.py +++ b/src/sirocco/parsing/_yaml_data_models.py @@ -417,14 +417,13 @@ class ConfigNamelist: - specs is a dictionnary containing the specifications of parameters to change in the original namelist file - For example: - - ... python + Example: - ConfigNamelist(path="/some/path/to/icon.nml", - specs={"first_nml_block":{"first_param": first_value, - "second_param": second_value}, - "second_nml_block":{"third_param": third_value}}) + >>> path="/some/path/to/icon.nml" + >>> specs = {"first_nml_block": {"first_param": "a string value", + ... "second_param": 0}, + ... "second_nml_block": {"third_param": False}} + >>> config_nml = ConfigNamelist(path=path, specs=specs) """ path: Path diff --git a/src/sirocco/pretty_print.py b/src/sirocco/pretty_print.py index baaf7ac5..7e219573 100644 --- a/src/sirocco/pretty_print.py +++ b/src/sirocco/pretty_print.py @@ -8,7 +8,7 @@ from sirocco import core -@dataclasses.dataclass +@dataclasses.dataclass(kw_only=True) class PrettyPrinter: """ Pretty print unrolled workflow graph elements in a reproducible and human readable format. From 95c263981e6deaa19d741accecb7f695ffe1a4f8 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 27 Jan 2025 16:16:38 +0100 Subject: [PATCH 4/8] fix: hatch fmt --- src/sirocco/parsing/_yaml_data_models.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sirocco/parsing/_yaml_data_models.py b/src/sirocco/parsing/_yaml_data_models.py index 37809aa1..20e2e53a 100644 --- a/src/sirocco/parsing/_yaml_data_models.py +++ b/src/sirocco/parsing/_yaml_data_models.py @@ -419,10 +419,11 @@ class ConfigNamelist: Example: - >>> path="/some/path/to/icon.nml" - >>> specs = {"first_nml_block": {"first_param": "a string value", - ... "second_param": 0}, - ... "second_nml_block": {"third_param": False}} + >>> path = "/some/path/to/icon.nml" + >>> specs = { + ... "first_nml_block": {"first_param": "a string value", "second_param": 0}, + ... "second_nml_block": {"third_param": False}, + ... } >>> config_nml = ConfigNamelist(path=path, specs=specs) """ From 2554524e4882bb1461779a06c5d6b8bf77ce7b6c Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 27 Jan 2025 16:19:06 +0100 Subject: [PATCH 5/8] fix: duplicate @dataclass strangely not caught by ruff --- src/sirocco/parsing/_yaml_data_models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sirocco/parsing/_yaml_data_models.py b/src/sirocco/parsing/_yaml_data_models.py index 20e2e53a..ab9426c6 100644 --- a/src/sirocco/parsing/_yaml_data_models.py +++ b/src/sirocco/parsing/_yaml_data_models.py @@ -473,7 +473,6 @@ class DataType(enum.StrEnum): DIR = enum.auto() -@dataclass @dataclass(kw_only=True) class ConfigBaseDataSpecs: type: DataType From 38e8e4f5db4ae3c59c875bfd9dcfeeca2727fe30 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 27 Jan 2025 17:46:11 +0100 Subject: [PATCH 6/8] add: ConfigIconTask doctest also make validator idempotent even if unused yet --- src/sirocco/parsing/_yaml_data_models.py | 35 +++++++++++++++++++----- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/src/sirocco/parsing/_yaml_data_models.py b/src/sirocco/parsing/_yaml_data_models.py index ab9426c6..0b81b266 100644 --- a/src/sirocco/parsing/_yaml_data_models.py +++ b/src/sirocco/parsing/_yaml_data_models.py @@ -438,18 +438,39 @@ class ConfigIconTaskSpecs: class ConfigIconTask(ConfigBaseTask, ConfigIconTaskSpecs): + """Class representing an ICON task configuration from a workflow file + + Examples: + + yaml snippet: + + >>> import textwrap + >>> import pydantic_yaml + >>> snippet = textwrap.dedent( + ... ''' + ... ICON: + ... plugin: icon + ... namelists: + ... - path/to/icon_master.namelist + ... - path/to/case_nml: + ... block_1: + ... param_name: param_value + ... ''' + ... ) + >>> icon_task_cfg = pydantic_yaml.parse_yaml_raw_as(ConfigIconTask, snippet) + """ @field_validator("namelists", mode="before") @classmethod - def check_nml(cls, nml_list: list[Any]) -> dict[str, ConfigNamelist]: - if nml_list is None: - msg = "ICON tasks need namelists, got none" - raise ValueError(msg) - if not isinstance(nml_list, list): - msg = f"expected a list got type {type(nml_list).__name__}" + def check_nmls(cls, nmls: dict[str, ConfigNamelist] | list[Any]) -> dict[str, ConfigNamelist]: + # Make validator idempotent even if not used yet + if isinstance(nmls, dict): + return nmls + if not isinstance(nmls, list): + msg = f"expected a list got type {type(nmls).__name__}" raise TypeError(msg) namelists = {} master_found = False - for nml in nml_list: + for nml in nmls: msg = f"was expecting a dict of length 1 or a string, got {nml}" if not isinstance(nml, (str, dict)): raise TypeError(msg) From 9aae9a0b5a11c9737554136fc089316332a1eaf8 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 27 Jan 2025 20:52:57 +0100 Subject: [PATCH 7/8] fix: hatch fmt --- src/sirocco/parsing/_yaml_data_models.py | 35 ++++++++++++------------ 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/sirocco/parsing/_yaml_data_models.py b/src/sirocco/parsing/_yaml_data_models.py index 0b81b266..5ad1153f 100644 --- a/src/sirocco/parsing/_yaml_data_models.py +++ b/src/sirocco/parsing/_yaml_data_models.py @@ -440,25 +440,26 @@ class ConfigIconTaskSpecs: class ConfigIconTask(ConfigBaseTask, ConfigIconTaskSpecs): """Class representing an ICON task configuration from a workflow file - Examples: - - yaml snippet: + Examples: - >>> import textwrap - >>> import pydantic_yaml - >>> snippet = textwrap.dedent( - ... ''' - ... ICON: - ... plugin: icon - ... namelists: - ... - path/to/icon_master.namelist - ... - path/to/case_nml: - ... block_1: - ... param_name: param_value - ... ''' - ... ) - >>> icon_task_cfg = pydantic_yaml.parse_yaml_raw_as(ConfigIconTask, snippet) + yaml snippet: + + >>> import textwrap + >>> import pydantic_yaml + >>> snippet = textwrap.dedent( + ... ''' + ... ICON: + ... plugin: icon + ... namelists: + ... - path/to/icon_master.namelist + ... - path/to/case_nml: + ... block_1: + ... param_name: param_value + ... ''' + ... ) + >>> icon_task_cfg = pydantic_yaml.parse_yaml_raw_as(ConfigIconTask, snippet) """ + @field_validator("namelists", mode="before") @classmethod def check_nmls(cls, nmls: dict[str, ConfigNamelist] | list[Any]) -> dict[str, ConfigNamelist]: From 819fc3c4bdd5d225da42aaa018319a8839486e62 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Tue, 28 Jan 2025 13:22:34 +0100 Subject: [PATCH 8/8] ref: remove useless None default --- src/sirocco/core/graph_items.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sirocco/core/graph_items.py b/src/sirocco/core/graph_items.py index d37df059..371089d9 100644 --- a/src/sirocco/core/graph_items.py +++ b/src/sirocco/core/graph_items.py @@ -39,7 +39,7 @@ class Data(ConfigBaseDataSpecs, GraphItem): color: ClassVar[str] = field(default="light_blue", repr=False) - available: bool | None = None # must get a default value because of dataclass inheritence + available: bool @classmethod def from_config(cls, config: ConfigBaseData, coordinates: dict) -> Self: