..

Author: dweindl

petab/v2/conditions.py

@@ -9,8 +9,8 @@
 import sympy as sp
 
 from .. import v2
+from ..v1.lint import assert_no_leading_trailing_whitespace
 from .C import *
-from .lint import assert_no_leading_trailing_whitespace
 
 __all__ = [
     "get_condition_df",

petab/v2/core.py

@@ -68,6 +68,15 @@ def _convert_nan_to_none(v):
     return v
 
 
+def _valid_petab_id(v: str) -> str:
+    """Field validator for PEtab IDs."""
+    if not v:
+        raise ValueError("ID must not be empty.")
+    if not is_valid_identifier(v):
+        raise ValueError(f"Invalid ID: {v}")
+    return v
+
+
 class ObservableTransformation(str, Enum):
     """Observable transformation types.
 
@@ -141,7 +150,9 @@ class Observable(BaseModel):
     """Observable definition."""
 
     #: Observable ID.
-    id: str = Field(alias=C.OBSERVABLE_ID)
+    id: Annotated[str, AfterValidator(_valid_petab_id)] = Field(
+        alias=C.OBSERVABLE_ID
+    )
     #: Observable name.
     name: str | None = Field(alias=C.OBSERVABLE_NAME, default=None)
     #: Observable formula.
@@ -162,15 +173,6 @@ class Observable(BaseModel):
         arbitrary_types_allowed=True, populate_by_name=True
     )
 
-    @field_validator("id")
-    @classmethod
-    def _validate_id(cls, v):
-        if not v:
-            raise ValueError("ID must not be empty.")
-        if not is_valid_identifier(v):
-            raise ValueError(f"Invalid ID: {v}")
-        return v
-
     @field_validator(
         "name",
         "formula",
@@ -313,9 +315,11 @@ class Change(BaseModel):
     """
 
     #: The ID of the target entity to change.
-    target_id: str | None = Field(alias=C.TARGET_ID, default=None)
+    target_id: Annotated[str, AfterValidator(_valid_petab_id)] = Field(
+        alias=C.TARGET_ID
+    )
     #: The value to set the target entity to.
-    target_value: sp.Basic | None = Field(alias=C.TARGET_VALUE, default=None)
+    target_value: sp.Basic = Field(alias=C.TARGET_VALUE)
 
     #: :meta private:
     model_config = ConfigDict(
@@ -324,16 +328,6 @@ class Change(BaseModel):
         use_enum_values=True,
     )
 
-    @model_validator(mode="before")
-    @classmethod
-    def _validate_id(cls, data: dict):
-        target_id = data.get("target_id", data.get(C.TARGET_ID))
-
-        if not is_valid_identifier(target_id):
-            raise ValueError(f"Invalid ID: {target_id}")
-
-        return data
-
     @field_validator("target_value", mode="before")
     @classmethod
     def _sympify(cls, v):
@@ -366,22 +360,15 @@ class Condition(BaseModel):
     """
 
     #: The condition ID.
-    id: str = Field(alias=C.CONDITION_ID)
+    id: Annotated[str, AfterValidator(_valid_petab_id)] = Field(
+        alias=C.CONDITION_ID
+    )
     #: The changes associated with this condition.
     changes: list[Change]
 
     #: :meta private:
     model_config = ConfigDict(populate_by_name=True)
 
-    @field_validator("id")
-    @classmethod
-    def _validate_id(cls, v):
-        if not v:
-            raise ValueError("ID must not be empty.")
-        if not is_valid_identifier(v):
-            raise ValueError(f"Invalid ID: {v}")
-        return v
-
     def __add__(self, other: Change) -> Condition:
         """Add a change to the set."""
         if not isinstance(other, Change):
@@ -488,8 +475,6 @@ class ExperimentPeriod(BaseModel):
     def _validate_id(cls, condition_id):
         if pd.isna(condition_id) or not condition_id:
             return None
-        # if not condition_id:
-        #     raise ValueError("ID must not be empty.")
         if not is_valid_identifier(condition_id):
             raise ValueError(f"Invalid ID: {condition_id}")
         return condition_id
@@ -504,7 +489,9 @@ class Experiment(BaseModel):
     """
 
     #: The experiment ID.
-    id: str = Field(alias=C.EXPERIMENT_ID)
+    id: Annotated[str, AfterValidator(_valid_petab_id)] = Field(
+        alias=C.EXPERIMENT_ID
+    )
     #: The periods of the experiment.
     periods: list[ExperimentPeriod] = []
 
@@ -513,15 +500,6 @@ class Experiment(BaseModel):
         arbitrary_types_allowed=True, populate_by_name=True
     )
 
-    @field_validator("id")
-    @classmethod
-    def _validate_id(cls, v):
-        if not v:
-            raise ValueError("ID must not be empty.")
-        if not is_valid_identifier(v):
-            raise ValueError(f"Invalid ID: {v}")
-        return v
-
     def __add__(self, other: ExperimentPeriod) -> Experiment:
         """Add a period to the experiment."""
         if not isinstance(other, ExperimentPeriod):
@@ -747,7 +725,9 @@ class Mapping(BaseModel):
     """Mapping PEtab entities to model entities."""
 
     #: PEtab entity ID.
-    petab_id: str = Field(alias=C.PETAB_ENTITY_ID)
+    petab_id: Annotated[str, AfterValidator(_valid_petab_id)] = Field(
+        alias=C.PETAB_ENTITY_ID
+    )
     #: Model entity ID.
     model_id: Annotated[str | None, BeforeValidator(_convert_nan_to_none)] = (
         Field(alias=C.MODEL_ENTITY_ID, default=None)
@@ -760,17 +740,6 @@ class Mapping(BaseModel):
     #: :meta private:
     model_config = ConfigDict(populate_by_name=True)
 
-    @field_validator(
-        "petab_id",
-    )
-    @classmethod
-    def _validate_id(cls, v):
-        if not v:
-            raise ValueError("ID must not be empty.")
-        if not is_valid_identifier(v):
-            raise ValueError(f"Invalid ID: {v}")
-        return v
-
 
 class MappingTable(BaseModel):
     """PEtab mapping table."""
@@ -913,7 +882,7 @@ def _validate(self) -> Self:
             )
 
         if self.lb is not None and self.ub is not None and self.lb >= self.ub:
-            raise ValueError("Lower bound must be less than upper bound")
+            raise ValueError("Lower bound must be less than upper bound.")
 
         # TODO parameterScale?
 

petab/v2/lint.py

@@ -14,8 +14,6 @@
 import sympy as sp
 
 from .. import v2
-from ..v1.visualize.lint import validate_visualization_df
-from ..v2.C import *
 from .problem import Problem
 
 logger = logging.getLogger(__name__)
@@ -165,7 +163,7 @@ def lint_problem(problem: Problem | str | Path) -> ValidationResultList:
     Arguments:
         problem:
             PEtab problem to check. Instance of :class:`Problem` or path
-            to a PEtab problem yaml file.
+            to a PEtab problem YAML file.
     Returns:
         A list of validation results. Empty if no issues were found.
     """
@@ -324,7 +322,7 @@ def run(self, problem: Problem) -> ValidationIssue | None:
 
 
 class CheckPosLogMeasurements(ValidationTask):
-    """A task to check that measurements for observables with
+    """Check that measurements for observables with
     log-transformation are positive."""
 
     def run(self, problem: Problem) -> ValidationIssue | None:
@@ -669,7 +667,9 @@ class CheckVisualizationTable(ValidationTask):
 
     def run(self, problem: Problem) -> ValidationIssue | None:
         if problem.visualization_df is None:
-            return
+            return None
+
+        from ..v1.visualize.lint import validate_visualization_df
 
         if validate_visualization_df(problem):
             return ValidationIssue(
@@ -698,22 +698,23 @@ def get_valid_parameters_for_parameter_table(
     # - remove parameters for which condition table columns exist
     # - remove placeholder parameters
     #   (only partial overrides are not supported)
+
     # must not go into parameter table
-    blackset = set(get_placeholders(problem))
+    invalid = set(get_placeholders(problem))
 
     # condition table targets
-    blackset |= {
+    invalid |= {
         change.target_id
         for cond in problem.conditions_table.conditions
         for change in cond.changes
     }
 
     # don't use sets here, to have deterministic ordering,
-    #  e.g. for creating parameter tables
+    #  e.g., for creating parameter tables
     parameter_ids = OrderedDict.fromkeys(
         p
         for p in problem.model.get_valid_parameters_for_parameter_table()
-        if p not in blackset
+        if p not in invalid
     )
 
     for mapping in problem.mapping_table.mappings:
@@ -723,14 +724,14 @@ def get_valid_parameters_for_parameter_table(
     # add output parameters from observables table
     output_parameters = get_output_parameters(problem)
     for p in output_parameters:
-        if p not in blackset:
+        if p not in invalid:
             parameter_ids[p] = None
 
     # Append parameters from measurement table, unless they occur as condition
     # table columns
     def append_overrides(overrides):
         for p in overrides:
-            if isinstance(p, sp.Symbol) and (str_p := str(p)) not in blackset:
+            if isinstance(p, sp.Symbol) and (str_p := str(p)) not in invalid:
                 parameter_ids[str_p] = None
 
     for measurement in problem.measurement_table.measurements:
@@ -932,4 +933,5 @@ def get_placeholders(
     CheckUnusedConditions(),
     # TODO: atomize checks, update to long condition table, re-enable
     # CheckVisualizationTable(),
+    # TODO validate mapping table
 ]