note todo and remove

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(

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.

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")