Remove TestPipeline and remove outdated example (#306)

* adjust docstrings a bit

* replace value errors with type errors where appropriate

* remove TestPipeline

* switch import format in a file

* clean up tests

* remove outdated example

eogrow/cli.py

@@ -14,7 +14,6 @@
 from .core.config import CrudeConfig, RawConfig, collect_configs_from_path, interpret_config_from_dict
 from .core.logging import CLUSTER_FILE_LOCATION_ON_HEAD
 from .core.schemas import build_schema_template
-from .pipelines.testing import TestPipeline
 from .utils.general import jsonify
 from .utils.meta import collect_schema, import_object, load_pipeline_class
 from .utils.pipeline_chain import run_pipeline_chain, validate_pipeline_chain
@@ -213,22 +212,6 @@ def validate_config(config_path: str) -> None:
     click.echo("Config validation succeeded!")
 
 
-@click.command()
-@click.argument("config_path", type=click.Path())
-def run_test_pipeline(config_path: str) -> None:
-    """Runs a test pipeline that only makes sure the managers work correctly. This can be used to select best
-    area manager parameters.
-
-    \b
-    Example:
-        eogrow-test any_pipeline_config.json
-    """
-    for crude_config in collect_configs_from_path(config_path):
-        raw_config = interpret_config_from_dict(crude_config)
-        pipeline = TestPipeline.with_defaults(raw_config)
-        pipeline.run()
-
-
 def _prepare_config(config: CrudeConfig, variables: dict[str, str], test_patches: Iterable[int]) -> RawConfig:
     raw_config = interpret_config_from_dict(config, variables)
     if test_patches:

eogrow/core/config.py

@@ -39,7 +39,7 @@ def collect_configs_from_path(path: str, used_config_paths: set[str] | None = No
     config = _recursive_config_build(config, used_config_paths)
 
     if not isinstance(config, (dict, list)):
-        raise ValueError(f"When interpreting config from {path} a dictionary or list was expected, got {type(config)}.")
+        raise TypeError(f"When interpreting config from {path} a dictionary or list was expected, got {type(config)}.")
     return cast(Union[CrudeConfig, List[CrudeConfig]], config)
 
 
@@ -57,7 +57,7 @@ def _recursive_config_build(config: object, used_config_paths: set[str]) -> obje
 
         for key, value in config.items():
             if not isinstance(key, str):
-                raise ValueError(f"Dictionary keys should always be strings, but found: {key}")
+                raise TypeError(f"Dictionary keys should always be strings, but found: {key}")
 
             if key.startswith("**"):
                 if value in used_config_paths:
@@ -89,6 +89,9 @@ def interpret_config_from_dict(config: CrudeConfig, external_variables: dict[str
     """
     _recursive_check_config(config)
 
+    if not isinstance(config, dict):
+        raise TypeError(f"Can only interpret dictionary objects, got {type(config)}.")
+
     config = cast(CrudeConfig, config.copy())
     variable_mapping = config.pop("variables", {})
     if external_variables:
@@ -101,10 +104,6 @@ def interpret_config_from_dict(config: CrudeConfig, external_variables: dict[str
     config_with_variables = _recursive_apply_to_strings(
         config, lambda config_str: _resolve_variables(config_str, variable_mapping)
     )
-    if not isinstance(config_with_variables, dict):
-        raise ValueError(
-            f"Interpretation resulted in object of type {type(config_with_variables)} but a dictionary was expected."
-        )
 
     return cast(RawConfig, config_with_variables)
 
@@ -153,14 +152,11 @@ def _recursive_apply_to_strings(config: object, function: Callable) -> object:
 
 
 def _recursive_check_config(config: object) -> None:
-    """Recursively checks if the config satisfies basic conditions for being JSON serializable.
-
-    :raises: ValueError
-    """
+    """Recursively checks if the config satisfies basic conditions for being JSON serializable."""
     if isinstance(config, dict):
         for key, value in config.items():
             if not isinstance(key, str):
-                raise ValueError(f"Config keys should be strings but {key} found")
+                raise TypeError(f"Config keys should be strings but {key} found")
             _recursive_check_config(value)
 
     elif isinstance(config, list):

eogrow/core/pipeline.py

@@ -81,7 +81,7 @@ def _new_pipeline_id() -> str:
     def _load_manager(manager_config: ManagerSchema, **manager_params: Any) -> Any:
         """Loads a manager class and back-propagates parsed config
 
-        :param manager_key: A config key name of a sub-config with manager parameters
+        :param manager_config: A sub-config with manager parameters
         :param manager_params: Other parameters to initialize a manager class
         """
         if manager_config.manager is None:
@@ -94,7 +94,7 @@ def get_pipeline_execution_name(self, pipeline_timestamp: str) -> str:
         return f"{pipeline_timestamp}-{self._pipeline_name}-{self.pipeline_id}"
 
     def get_patch_list(self) -> PatchList:
-        """Method which at the initialization prepares the list of EOPatches which will be used"""
+        """Method that prepares the list of EOPatches for which to run the pipeline execution."""
         patch_list = self.area_manager.get_patch_list()
 
         if self.config.test_subset is not None:
@@ -130,7 +130,7 @@ def get_execution_arguments(self, workflow: EOWorkflow, patch_list: PatchList) -
         """Prepares execution arguments for each eopatch from a list of patches.
 
         The output should be a dictionary of form `{execution_name: {node: node_kwargs}}`. Execution names are usually
-         names of EOPatches, but can be anything.
+        names of EOPatches, but can be anything.
 
         :param workflow: A workflow for which arguments will be prepared
         """
@@ -262,8 +262,7 @@ def run_procedure(self) -> tuple[list[str], list[str]]:
         """
         if not hasattr(self, "build_workflow"):
             raise NotImplementedError(
-                "Default implementation of the `run_procedure` method requires implementation of the `build_workflow`"
-                " method."
+                "Implementation of the `run_procedure` method requires implementation of the `build_workflow` method."
             )
         workflow = self.build_workflow()
         patch_list = self.get_patch_list()

eogrow/pipelines/export_maps.py

@@ -131,7 +131,7 @@ def run_procedure(self) -> tuple[list[str], list[str]]:
         successful, failed, _ = self.run_execution(workflow, exec_args)
 
         if not successful:
-            raise ValueError("Failed to extract tiff files from any of EOPatches.")
+            raise RuntimeError("Failed to extract tiff files from any of EOPatches.")
 
         feature_type, _ = self.config.feature
         output_folder = self.storage.get_folder(self.config.output_folder_key)

eogrow/pipelines/sampling.py

@@ -92,7 +92,7 @@ def _get_loading_node(self) -> EONode:
                 feature_type = FeatureType(feature_type_str)
 
                 if not feature_type.is_spatial():
-                    raise ValueError(f"Only spatial features can be sampled, but found {feature_type}: {feature_names}")
+                    raise TypeError(f"Only spatial features can be sampled, but found {feature_type}: {feature_names}")
 
                 for feature_name in feature_names:
                     load_features.append((feature_type, feature_name))  # noqa: PERF401

eogrow/pipelines/testing.py

@@ -2,67 +2,20 @@
 
 from __future__ import annotations
 
-import logging
-from typing import List, Literal, Optional, Tuple, TypeVar, Union
+from typing import List, Literal, Optional, Tuple, Union
 
 import numpy as np
 from pydantic import Field
 
 from eolearn.core import CreateEOPatchTask, EONode, EOWorkflow, OverwritePermission, SaveTask
 from eolearn.core.types import Feature
 
-from ..core.config import RawConfig, recursive_config_join
 from ..core.pipeline import Pipeline
 from ..core.schemas import BaseSchema
-from ..tasks.testing import (
-    GenerateRasterFeatureTask,
-    GenerateTimestampsTask,
-    NormalDistribution,
-    UniformDistribution,
-)
+from ..tasks.testing import GenerateRasterFeatureTask, GenerateTimestampsTask, NormalDistribution, UniformDistribution
 from ..types import ExecKwargs, PatchList, TimePeriod
 from ..utils.validators import ensure_storage_key_presence, field_validator, parse_dtype, parse_time_period
 
-Self = TypeVar("Self", bound="TestPipeline")
-LOGGER = logging.getLogger(__name__)
-
-
-class TestPipeline(Pipeline):
-    """Pipeline that just tests if all managers works correctly. It can be used to check if area manager creates a
-    correct grid.
-    """
-
-    class Schema(Pipeline.Schema):
-        class Config:
-            extra = "allow"
-
-    _DEFAULT_CONFIG_PARAMS = {  # noqa: RUF012
-        "pipeline": "eogrow.pipelines.testing.TestPipeline",
-        "logging": {"manager": "eogrow.logging.LoggingManager", "show_logs": True},
-    }
-
-    @classmethod
-    def with_defaults(cls: type[Self], config: RawConfig) -> Self:
-        config = recursive_config_join(config, cls._DEFAULT_CONFIG_PARAMS)  # type: ignore[assignment]
-        return cls.from_raw_config(config)
-
-    def run_procedure(self) -> tuple[list, list]:
-        """Performs basic tests of managers"""
-        if self.storage.filesystem.exists("/"):
-            LOGGER.info("Project folder %s exists", self.storage.config.project_folder)
-        else:
-            LOGGER.info("Project folder %s does not exist", self.storage.config.project_folder)
-
-        self.area_manager.get_area_geometry()
-        grid = self.area_manager.get_grid()
-        num_patches = sum(map(len, grid.values()))
-        LOGGER.info("Grid has %d EOPatches and is split over %d CRS zones", num_patches, len(grid))
-
-        patch_list = self.area_manager.get_patch_list()
-        LOGGER.info("The first EOPatch has a name %s", patch_list[0][0])
-
-        return [], []
-
 
 class UniformDistributionSchema(BaseSchema):
     kind: Literal["uniform"]