no conditionName, no operationType

Author: dweindl

petab/v1/math/sympify.py

@@ -31,9 +31,13 @@ def sympify_petab(expr: str | int | float) -> sp.Expr | sp.Basic:
     if isinstance(expr, float) or isinstance(expr, np.floating):
         return sp.Float(expr)
 
-    # Set error listeners
-    input_stream = InputStream(expr)
+    try:
+        input_stream = InputStream(expr)
+    except TypeError as e:
+        raise TypeError(f"Error parsing {expr!r}: {e.args[0]}") from e
+
     lexer = PetabMathExprLexer(input_stream)
+    # Set error listeners
     lexer.removeErrorListeners()
     lexer.addErrorListener(MathErrorListener())
 

petab/v2/C.py

@@ -125,28 +125,14 @@
 
 #: Condition ID column in the condition table
 CONDITION_ID = "conditionId"
-# TODO: removed?
-#: Condition name column in the condition table
-CONDITION_NAME = "conditionName"
 #: Column in the condition table with the ID of an entity that is changed
 TARGET_ID = "targetId"
-#: Column in the condition table with the operation type
-OPERATION_TYPE = "operationType"
 #: Column in the condition table with the new value of the target entity
 TARGET_VALUE = "targetValue"
-# operation types:
-OT_CUR_VAL = "setCurrentValue"
-OT_NO_CHANGE = "noChange"
-
-OPERATION_TYPES = [
-    OT_CUR_VAL,
-    OT_NO_CHANGE,
-]
 
 CONDITION_DF_COLS = [
     CONDITION_ID,
     TARGET_ID,
-    OPERATION_TYPE,
     TARGET_VALUE,
 ]
 

petab/v2/core.py

@@ -2,13 +2,16 @@
 
 from __future__ import annotations
 
+from collections.abc import Sequence
 from enum import Enum
 from pathlib import Path
+from typing import Annotated
 
 import numpy as np
 import pandas as pd
 import sympy as sp
 from pydantic import (
+    AfterValidator,
     BaseModel,
     ConfigDict,
     Field,
@@ -29,7 +32,6 @@
     "Change",
     "Condition",
     "ConditionsTable",
-    "OperationType",
     "ExperimentPeriod",
     "Experiment",
     "ExperimentsTable",
@@ -43,6 +45,14 @@
 ]
 
 
+def is_finite_or_neg_inf(v: float, info: ValidationInfo) -> float:
+    if not np.isfinite(v) and v != -np.inf:
+        raise ValueError(
+            f"{info.field_name} value must be finite or -inf but got {v}"
+        )
+    return v
+
+
 class ObservableTransformation(str, Enum):
     """Observable transformation types.
 
@@ -248,16 +258,6 @@ def __iadd__(self, other: Observable) -> ObservablesTable:
         return self
 
 
-# TODO remove?!
-class OperationType(str, Enum):
-    """Operation types for model changes in the PEtab conditions table."""
-
-    # TODO update names
-    SET_CURRENT_VALUE = "setCurrentValue"
-    NO_CHANGE = "noChange"
-    ...
-
-
 class Change(BaseModel):
     """A change to the model or model state.
 
@@ -266,17 +266,13 @@ class Change(BaseModel):
 
     >>> Change(
     ...     target_id="k1",
-    ...     operation_type=OperationType.SET_CURRENT_VALUE,
     ...     target_value="10",
     ... )  # doctest: +NORMALIZE_WHITESPACE
-    Change(target_id='k1', operation_type='setCurrentValue',
-    target_value=10.0000000000000)
+    Change(target_id='k1', target_value=10.0000000000000)
     """
 
     #: The ID of the target entity to change.
     target_id: str | None = Field(alias=C.TARGET_ID, default=None)
-    # TODO: remove?!
-    operation_type: OperationType = Field(alias=C.OPERATION_TYPE)
     #: The value to set the target entity to.
     target_value: sp.Basic | None = Field(alias=C.TARGET_VALUE, default=None)
 
@@ -290,14 +286,11 @@ class Change(BaseModel):
     @model_validator(mode="before")
     @classmethod
     def _validate_id(cls, data: dict):
-        if (
-            data.get("operation_type", data.get(C.OPERATION_TYPE))
-            != C.OT_NO_CHANGE
-        ):
-            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}")
+        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")
@@ -323,13 +316,12 @@ class Condition(BaseModel):
     ...     changes=[
     ...         Change(
     ...             target_id="k1",
-    ...             operation_type=OperationType.SET_CURRENT_VALUE,
     ...             target_value="10",
     ...         )
     ...     ],
     ... )  # doctest: +NORMALIZE_WHITESPACE
-    Condition(id='condition1', changes=[Change(target_id='k1',
-    operation_type='setCurrentValue', target_value=10.0000000000000)])
+    Condition(id='condition1',
+    changes=[Change(target_id='k1', target_value=10.0000000000000)])
     """
 
     #: The condition ID.
@@ -352,13 +344,13 @@ def _validate_id(cls, v):
     def __add__(self, other: Change) -> Condition:
         """Add a change to the set."""
         if not isinstance(other, Change):
-            raise TypeError("Can only add Change to ChangeSet")
+            raise TypeError("Can only add Change to Condition")
         return Condition(id=self.id, changes=self.changes + [other])
 
     def __iadd__(self, other: Change) -> Condition:
         """Add a change to the set in place."""
         if not isinstance(other, Change):
-            raise TypeError("Can only add Change to ChangeSet")
+            raise TypeError("Can only add Change to Condition")
         self.changes.append(other)
         return self
 
@@ -379,11 +371,11 @@ def __getitem__(self, condition_id: str) -> Condition:
     @classmethod
     def from_df(cls, df: pd.DataFrame) -> ConditionsTable:
         """Create a ConditionsTable from a DataFrame."""
-        if df is None:
+        if df is None or df.empty:
             return cls(conditions=[])
 
         conditions = []
-        for condition_id, sub_df in df.groupby(C.CONDITION_ID):
+        for condition_id, sub_df in df.reset_index().groupby(C.CONDITION_ID):
             changes = [Change(**row.to_dict()) for _, row in sub_df.iterrows()]
             conditions.append(Condition(id=condition_id, changes=changes))
 
@@ -422,13 +414,13 @@ def to_tsv(self, file_path: str | Path) -> None:
     def __add__(self, other: Condition) -> ConditionsTable:
         """Add a condition to the table."""
         if not isinstance(other, Condition):
-            raise TypeError("Can only add ChangeSet to ConditionsTable")
+            raise TypeError("Can only add Conditions to ConditionsTable")
         return ConditionsTable(conditions=self.conditions + [other])
 
     def __iadd__(self, other: Condition) -> ConditionsTable:
         """Add a condition to the table in place."""
         if not isinstance(other, Condition):
-            raise TypeError("Can only add ChangeSet to ConditionsTable")
+            raise TypeError("Can only add Conditions to ConditionsTable")
         self.conditions.append(other)
         return self
 
@@ -441,21 +433,20 @@ class ExperimentPeriod(BaseModel):
     """
 
     #: The start time of the period in time units as defined in the model.
-    # TODO: Only finite times and -inf are allowed as start time
-    time: float = Field(alias=C.TIME)
-    # TODO: decide if optional
+    time: Annotated[float, AfterValidator(is_finite_or_neg_inf)] = Field(
+        alias=C.TIME
+    )
     #: The ID of the condition to be applied at the start time.
-    condition_id: str = Field(alias=C.CONDITION_ID)
+    condition_id: str | None = Field(alias=C.CONDITION_ID, default=None)
 
     #: :meta private:
     model_config = ConfigDict(populate_by_name=True)
 
     @field_validator("condition_id", mode="before")
     @classmethod
     def _validate_id(cls, condition_id):
-        # TODO to be decided if optional
-        if pd.isna(condition_id):
-            return ""
+        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):
@@ -633,12 +624,17 @@ def _validate_id(cls, v, info: ValidationInfo):
     )
     @classmethod
     def _sympify_list(cls, v):
+        if v is None:
+            return []
+
         if isinstance(v, float) and np.isnan(v):
             return []
+
         if isinstance(v, str):
             v = v.split(C.PARAMETER_SEPARATOR)
-        else:
+        elif not isinstance(v, Sequence):
             v = [v]
+
         return [sympify_petab(x) for x in v]
 
 

petab/v2/lint.py

@@ -518,13 +518,13 @@ class CheckExperimentConditionsExist(ValidationTask):
     def run(self, problem: Problem) -> ValidationIssue | None:
         messages = []
         available_conditions = {
-            c.id
-            for c in problem.conditions_table.conditions
-            if not pd.isna(c.id)
+            c.id for c in problem.conditions_table.conditions
         }
         for experiment in problem.experiments_table.experiments:
             missing_conditions = {
-                period.condition_id for period in experiment.periods
+                period.condition_id
+                for period in experiment.periods
+                if period.condition_id is not None
             } - available_conditions
             if missing_conditions:
                 messages.append(
@@ -672,7 +672,7 @@ def get_valid_parameters_for_parameter_table(
         blackset |= placeholders
 
     if condition_df is not None:
-        blackset |= set(condition_df.columns.values) - {CONDITION_NAME}
+        blackset |= set(condition_df.columns.values)
 
     # don't use sets here, to have deterministic ordering,
     #  e.g. for creating parameter tables

petab/v2/petab1to2.py

@@ -226,8 +226,9 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str:
     validation_issues = v2.lint_problem(new_yaml_file)
 
     if validation_issues:
+        validation_issues = "\n".join(map(str, validation_issues))
         raise ValueError(
-            "Generated PEtab v2 problem did not pass linting: "
+            "The generated PEtab v2 problem did not pass linting: "
             f"{validation_issues}"
         )
 
@@ -287,7 +288,7 @@ def v1v2_condition_df(
     condition_df = condition_df.copy().reset_index()
     with suppress(KeyError):
         # conditionName was dropped in PEtab v2
-        condition_df.drop(columns=[v2.C.CONDITION_NAME], inplace=True)
+        condition_df.drop(columns=[v1.C.CONDITION_NAME], inplace=True)
 
     condition_df = condition_df.melt(
         id_vars=[v1.C.CONDITION_ID],
@@ -301,7 +302,6 @@ def v1v2_condition_df(
             columns=[
                 v2.C.CONDITION_ID,
                 v2.C.TARGET_ID,
-                v2.C.OPERATION_TYPE,
                 v2.C.TARGET_VALUE,
             ]
         )
@@ -320,5 +320,4 @@ def v1v2_condition_df(
                 f"Unable to determine value type {target} in the condition "
                 "table."
             )
-    condition_df[v2.C.OPERATION_TYPE] = v2.C.OT_CUR_VAL
     return condition_df