Merge remote-tracking branch 'origin/main' into claude/issue-531-placeholder-sort-5c5d23

# Conflicts:
#	petab/v2/petab1to2.py

Author: FFroehlich

petab/v2/petab1to2.py

@@ -14,15 +14,19 @@
 from pandas.io.common import get_handle, is_url
 
 from .. import v1, v2
-from ..v1.yaml import get_path_prefix, load_yaml, validate
+from ..v1.yaml import get_path_prefix, load_yaml
+from ..v1.yaml import validate as validate_yaml
 from ..versions import get_major_version
 from .models import MODEL_TYPE_SBML
 
 __all__ = ["petab1to2"]
 
 
 def petab1to2(
-    yaml_config: Path | str, output_dir: Path | str | None = None
+    yaml_config: Path | str,
+    output_dir: Path | str | None = None,
+    *,
+    validate: bool = True,
 ) -> v2.Problem | None:
     """Convert from PEtab 1.0 to PEtab 2.0 format.
 
@@ -44,31 +48,45 @@ def petab1to2(
     :param output_dir:
         The output directory to save the converted PEtab problem, or ``None``,
         to return a :class:`petab.v2.Problem` instance.
+    :param validate:
+        Whether to lint the input PEtab v1 problem, and the
+        output PEtab v2 problem.
+        If ``True`` (default), both problems are validated.
+        If ``False``, neither problem is validated.
+        The ``yaml_config`` content is validated regardless.
 
     :raises ValueError:
-        If the input is invalid or does not pass linting or if the generated
-        files do not pass linting.
+        If the input is invalid or, if ``validate`` is ``True``, if the input
+        does not pass linting or if the generated files do not pass linting.
     """
     if output_dir is not None:
-        return petab_files_1to2(yaml_config, output_dir)
+        return petab_files_1to2(yaml_config, output_dir, validate=validate)
 
     with TemporaryDirectory() as tmp_dir:
-        petab_files_1to2(yaml_config, tmp_dir)
+        petab_files_1to2(yaml_config, tmp_dir, validate=validate)
         return v2.Problem.from_yaml(Path(tmp_dir, Path(yaml_config).name))
 
 
-def petab_files_1to2(yaml_config: Path | str | dict, output_dir: Path | str):
+def petab_files_1to2(
+    yaml_config: Path | str | dict,
+    output_dir: Path | str,
+    *,
+    validate: bool = True,
+):
     """Convert PEtab files from PEtab 1.0 to PEtab 2.0.
 
 
     :param yaml_config:
         The PEtab problem as dictionary or YAML file name.
     :param output_dir:
         The output directory to save the converted PEtab problem.
+    :param validate:
+        Whether to lint the PEtab v1 problem before and the generated
+        PEtab v2 problem after the conversion. See :func:`petab1to2`.
 
     :raises ValueError:
-        If the input is invalid or does not pass linting or if the generated
-        files do not pass linting.
+        If the input is invalid or, if ``validate`` is ``True``, if the input
+        does not pass linting or if the generated files do not pass linting.
     """
     if isinstance(yaml_config, Path | str):
         yaml_file = str(yaml_config)
@@ -83,7 +101,7 @@ def petab_files_1to2(yaml_config: Path | str | dict, output_dir: Path | str):
     get_dest_path = lambda filename: f"{output_dir}/{filename}"  # noqa: E731
 
     # Validate the original PEtab problem
-    validate(yaml_config, path_prefix=path_prefix)
+    validate_yaml(yaml_config, path_prefix=path_prefix)
     if get_major_version(yaml_config) != 1:
         raise ValueError("PEtab problem is not version 1.")
     petab_problem = v1.Problem.from_yaml(yaml_file or yaml_config)
@@ -92,7 +110,7 @@ def petab_files_1to2(yaml_config: Path | str | dict, output_dir: Path | str):
     petab_problem.condition_df = petab_problem.condition_df.drop(
         columns=[v1.C.CONDITION_NAME], errors="ignore"
     )
-    if v1.lint_problem(petab_problem):
+    if validate and v1.lint_problem(petab_problem):
         raise ValueError("Provided PEtab problem does not pass linting.")
 
     output_dir = Path(output_dir)
@@ -132,20 +150,23 @@ def petab_files_1to2(yaml_config: Path | str | dict, output_dir: Path | str):
     # records for the experiment table to be created
     experiments = []
 
+    def v2_condition_id(cond_id: str) -> str:
+        """Get the condition ID to use in the v2 experiment table.
+
+        Conditions without any changes (all-NaN rows) are not included in the
+        v2 condition table. Periods applying such a condition get an empty
+        condition ID ("use the model as is").
+        """
+        if petab_problem.condition_df.loc[cond_id].notna().any():
+            return cond_id
+        return ""
+
     def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str:
         if not sim_cond_id and not preeq_cond_id:
             return ""
-        # check whether the conditions will exist in the v2 condition table
-        sim_cond_exists = (
-            petab_problem.condition_df.loc[sim_cond_id].notna().any()
-        )
-        preeq_cond_exists = (
-            preeq_cond_id
-            and petab_problem.condition_df.loc[preeq_cond_id].notna().any()
-        )
-        if not sim_cond_exists and not preeq_cond_exists:
-            # if we have only all-NaN conditions, we don't create a new
-            #  experiment
+        if not preeq_cond_id and not v2_condition_id(sim_cond_id):
+            # without pre-equilibration and without any changes, there is
+            #  nothing to be described by an experiment
             return ""
 
         if preeq_cond_id:
@@ -177,14 +198,14 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str:
                 {
                     v2.C.EXPERIMENT_ID: exp_id,
                     v2.C.TIME: v2.C.TIME_PREEQUILIBRATION,
-                    v2.C.CONDITION_ID: preeq_cond_id,
+                    v2.C.CONDITION_ID: v2_condition_id(preeq_cond_id),
                 }
             )
         experiments.append(
             {
                 v2.C.EXPERIMENT_ID: exp_id,
                 v2.C.TIME: 0,
-                v2.C.CONDITION_ID: sim_cond_id,
+                v2.C.CONDITION_ID: v2_condition_id(sim_cond_id),
             }
         )
     if experiments:
@@ -215,20 +236,6 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str:
         else:
             measurement_df[v1.C.PREEQUILIBRATION_CONDITION_ID] = ""
 
-        if (
-            petab_problem.condition_df is not None
-            and len(
-                set(petab_problem.condition_df.columns) - {v1.C.CONDITION_NAME}
-            )
-            == 0
-        ):
-            # we can't have "empty" conditions with no overrides in v2,
-            #  therefore, we drop the respective condition ID completely
-            #   TODO: or can we?
-            # TODO: this needs to be checked condition-wise, not globally
-            measurement_df[v1.C.SIMULATION_CONDITION_ID] = ""
-            if v1.C.PREEQUILIBRATION_CONDITION_ID in measurement_df.columns:
-                measurement_df[v1.C.PREEQUILIBRATION_CONDITION_ID] = ""
         # condition IDs to experiment IDs
         measurement_df.insert(
             0,
@@ -251,6 +258,9 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str:
     new_yaml_file = output_dir / Path(yaml_file).name
     new_yaml_config.to_yaml(new_yaml_file)
 
+    if not validate:
+        return
+
     # validate updated Problem
     validation_issues = v2.lint_problem(new_yaml_file)
 

tests/v2/test_conversion.py

@@ -1,9 +1,11 @@
 import logging
+from pathlib import Path
 
 import pandas as pd
 import pytest
 
 from petab import v1, v2
+from petab.v1.models.sbml_model import SbmlModel
 from petab.v2 import Problem
 from petab.v2.petab1to2 import petab1to2, v1v2_observable_df
 
@@ -148,6 +150,201 @@ def test_petab1to2_remote():
     assert len(problem.measurements)
 
 
+def _petab1to2_with_conditions(
+    tmp_path, condition_df: pd.DataFrame, conditions: list[tuple[str, str]]
+) -> Problem:
+    """Convert a minimal PEtab v1 problem with the given condition table.
+
+    ``conditions`` are (preequilibrationConditionId, simulationConditionId)
+    pairs, one measurement per pair.
+    """
+    problem = v1.Problem(
+        model=SbmlModel.from_antimony(
+            "A = 1; B = 0; k1 = 0.5; A -> B; k1 * A"
+        ),
+        condition_df=v1.get_condition_df(condition_df),
+        observable_df=v1.get_observable_df(
+            pd.DataFrame(
+                {
+                    v1.C.OBSERVABLE_ID: ["obs_B"],
+                    v1.C.OBSERVABLE_FORMULA: ["B"],
+                    v1.C.NOISE_FORMULA: [0.1],
+                }
+            )
+        ),
+        measurement_df=v1.get_measurement_df(
+            pd.DataFrame(
+                {
+                    v1.C.OBSERVABLE_ID: "obs_B",
+                    v1.C.PREEQUILIBRATION_CONDITION_ID: [
+                        preeq for preeq, _ in conditions
+                    ],
+                    v1.C.SIMULATION_CONDITION_ID: [
+                        sim for _, sim in conditions
+                    ],
+                    v1.C.TIME: 1.0,
+                    v1.C.MEASUREMENT: 0.5,
+                }
+            )
+        ),
+        parameter_df=v1.get_parameter_df(
+            pd.DataFrame(
+                {
+                    v1.C.PARAMETER_ID: ["k1"],
+                    v1.C.PARAMETER_SCALE: [v1.C.LIN],
+                    v1.C.LOWER_BOUND: [1e-3],
+                    v1.C.UPPER_BOUND: [1e3],
+                    v1.C.NOMINAL_VALUE: [0.5],
+                    v1.C.ESTIMATE: [1],
+                }
+            )
+        ),
+    )
+    return petab1to2(problem.to_files_generic(tmp_path))
+
+
+def _measured_periods(
+    problem: Problem,
+) -> list[list[tuple[float, list[str]]] | None]:
+    """The (time, conditionIds) of the experiment periods per measurement."""
+    return [
+        None
+        if m.experiment_id is None
+        else [
+            (p.time, p.condition_ids) for p in problem[m.experiment_id].periods
+        ]
+        for m in problem.measurements
+    ]
+
+
+def test_petab1to2_empty_conditions_preequilibration(tmp_path):
+    """Pre-equilibration is kept if the condition table sets nothing."""
+    problem = _petab1to2_with_conditions(
+        tmp_path,
+        pd.DataFrame({v1.C.CONDITION_ID: ["c0", "steady"]}),
+        [("steady", "c0")],
+    )
+
+    assert problem.conditions == []
+    assert _measured_periods(problem) == [
+        [(v2.C.TIME_PREEQUILIBRATION, []), (0, [])]
+    ]
+    assert problem.experiments[0].has_preequilibration
+
+
+def test_petab1to2_mixed_empty_conditions(tmp_path):
+    """Conditions that set nothing are handled condition-wise."""
+    problem = _petab1to2_with_conditions(
+        tmp_path,
+        pd.DataFrame(
+            {
+                v1.C.CONDITION_ID: ["c0", "c1", "preeq_set", "preeq_empty"],
+                "A": [1.0, None, 2.0, None],
+            }
+        ),
+        [
+            ("preeq_set", "c0"),
+            ("preeq_set", "c1"),
+            ("preeq_empty", "c0"),
+            ("preeq_empty", "c1"),
+            ("", "c0"),
+            ("", "c1"),
+        ],
+    )
+
+    assert {c.id for c in problem.conditions} == {"c0", "preeq_set"}
+    preeq = v2.C.TIME_PREEQUILIBRATION
+    assert _measured_periods(problem) == [
+        [(preeq, ["preeq_set"]), (0, ["c0"])],
+        [(preeq, ["preeq_set"]), (0, [])],
+        [(preeq, []), (0, ["c0"])],
+        [(preeq, []), (0, [])],
+        [(0, ["c0"])],
+        # no pre-equilibration, no changes -> no experiment
+        None,
+    ]
+
+
+def test_petab1to2_empty_conditions_no_preequilibration(tmp_path):
+    """Without pre-equilibration, conditions that set nothing don't need an
+    experiment."""
+    problem = _petab1to2_with_conditions(
+        tmp_path,
+        pd.DataFrame({v1.C.CONDITION_ID: ["c0"]}),
+        [("", "c0")],
+    )
+
+    assert problem.experiments == []
+    assert _measured_periods(problem) == [None]
+
+
+def test_petab1to2_validate(tmp_path):
+    """Test that `validate=False` allows converting problems that fail
+    linting for reasons unrelated to the conversion."""
+    v1_problem = v1.Problem(
+        model=SbmlModel.from_antimony(
+            "A -> B; k1 * A; A = 1; B = 0; k1 = 0.5"
+        ),
+        condition_df=v1.get_condition_df(
+            pd.DataFrame({v1.C.CONDITION_ID: ["c0"], "A": [1.0]})
+        ),
+        observable_df=v1.get_observable_df(
+            pd.DataFrame(
+                {
+                    v1.C.OBSERVABLE_ID: ["obs_B"],
+                    v1.C.OBSERVABLE_FORMULA: ["B"],
+                    v1.C.NOISE_FORMULA: [0.1],
+                }
+            )
+        ),
+        measurement_df=v1.get_measurement_df(
+            pd.DataFrame(
+                {
+                    v1.C.OBSERVABLE_ID: ["obs_B", "obs_B"],
+                    v1.C.SIMULATION_CONDITION_ID: ["c0", "c0"],
+                    v1.C.TIME: [1.0, 2.0],
+                    v1.C.MEASUREMENT: [0.3, 0.5],
+                    v1.C.DATASET_ID: ["d1", "d1"],
+                }
+            )
+        ),
+        parameter_df=v1.get_parameter_df(
+            pd.DataFrame(
+                {
+                    v1.C.PARAMETER_ID: ["k1"],
+                    v1.C.PARAMETER_SCALE: [v1.C.LIN],
+                    v1.C.LOWER_BOUND: [1e-3],
+                    v1.C.UPPER_BOUND: [1e3],
+                    v1.C.NOMINAL_VALUE: [0.5],
+                    v1.C.ESTIMATE: [1],
+                }
+            )
+        ),
+        # refers to a dataset that is not in the measurement table
+        visualization_df=pd.DataFrame(
+            {
+                v1.C.PLOT_ID: ["p1"],
+                v1.C.DATASET_ID: ["no_such_dataset"],
+                v1.C.X_VALUES: [v1.C.TIME],
+            }
+        ),
+    )
+    yaml_file = v1_problem.to_files_generic(tmp_path / "v1")
+
+    with pytest.raises(ValueError, match="does not pass linting"):
+        petab1to2(yaml_file)
+
+    problem = petab1to2(yaml_file, validate=False)
+    assert isinstance(problem, Problem)
+    assert len(problem.measurements) == 2
+
+    output_dir = tmp_path / "v2"
+    output_dir.mkdir()
+    petab1to2(yaml_file, output_dir, validate=False)
+    problem = Problem.from_yaml(output_dir / Path(yaml_file).name)
+    assert len(problem.measurements) == 2
+
+
 try:
     import benchmark_models_petab