Fix override check and add noise formula check (#48)

* fix timepoint specific mappings and make more flexible

* add check for nontrivial noise formulas

* fix pep

* add test

* fixup

* Update petab/lint.py

Co-authored-by: Daniel Weindl <[email protected]>

* fix according to review

* fix and add more tests

* add test explainers

Co-authored-by: Daniel Weindl <[email protected]>

Author: FFroehlich

petab/lint.py

@@ -4,7 +4,7 @@
 import logging
 import numbers
 import re
-from typing import Optional, Iterable, Union
+from typing import Optional, Iterable, Any
 from collections import Counter
 
 import libsbml
@@ -527,67 +527,103 @@ def assert_parameter_estimate_is_boolean(parameter_df: pd.DataFrame) -> None:
 
 
 def measurement_table_has_timepoint_specific_mappings(
-        measurement_df: pd.DataFrame) -> bool:
+        measurement_df: pd.DataFrame,
+        allow_scalar_numeric_noise_parameters: bool = False,
+        allow_scalar_numeric_observable_parameters: bool = False,
+) -> bool:
     """
     Are there time-point or replicate specific parameter assignments in the
     measurement table.
 
     Arguments:
-        measurement_df: PEtab measurement table
+        measurement_df:
+            PEtab measurement table
+
+        allow_scalar_numeric_noise_parameters:
+            ignore scalar numeric assignments to noiseParamater placeholders
+
+        allow_scalar_numeric_observable_parameters:
+            ignore scalar numeric assignments to observableParamater
+            placeholders
 
     Returns:
-        True if there are time-point or replicate specific parameter
-        assignments in the measurement table, False otherwise.
+        True if there are time-point or replicate specific (non-numeric)
+        parameter assignments in the measurement table, False otherwise.
     """
     # since we edit it, copy it first
     measurement_df = copy.deepcopy(measurement_df)
 
-    def is_numeric(x: Union[str, numbers.Number]) -> bool:
+    def is_scalar_float(x: Any):
         """
-        Checks whether x can be transformed into a (list of) float(s)
+        Checks whether input is a number or can be transformed into a number
+        via float
         :param x:
-            number or string containing numbers seperated by ;
+            input
         :return:
-            True if conversion is possible for all values
+            True if is or can be converted to number, False otherwise.
         """
         if isinstance(x, numbers.Number):
             return True
-        if not isinstance(x, str):
-            return False
         try:
-            [float(y) for y in x.split(';')]
+            float(x)
             return True
         except (ValueError, TypeError):
             return False
 
     # mask numeric values
-    for col in [OBSERVABLE_PARAMETERS, NOISE_PARAMETERS]:
+    for col, allow_scalar_numeric in [
+        (OBSERVABLE_PARAMETERS, allow_scalar_numeric_observable_parameters),
+        (NOISE_PARAMETERS, allow_scalar_numeric_noise_parameters)
+    ]:
         if col not in measurement_df:
             continue
-        measurement_df.loc[measurement_df[col].apply(is_numeric), col] = np.nan
+
+        measurement_df[col] = measurement_df[col].apply(str)
+
+        if allow_scalar_numeric:
+            measurement_df.loc[
+                measurement_df[col].apply(is_scalar_float), col
+            ] = np.nan
 
     grouping_cols = core.get_notnull_columns(
         measurement_df,
         [OBSERVABLE_ID,
          SIMULATION_CONDITION_ID,
          PREEQUILIBRATION_CONDITION_ID,
          OBSERVABLE_PARAMETERS,
-         NOISE_PARAMETERS,
-         ])
-    grouped_df = measurement_df.groupby(grouping_cols,
-                                        dropna=False).size().reset_index()
+         NOISE_PARAMETERS])
+    grouped_df = measurement_df.groupby(grouping_cols, dropna=False)
 
     grouping_cols = core.get_notnull_columns(
-        grouped_df,
+        measurement_df,
         [OBSERVABLE_ID,
          SIMULATION_CONDITION_ID,
          PREEQUILIBRATION_CONDITION_ID])
-    grouped_df2 = grouped_df.groupby(grouping_cols).size().reset_index()
+    grouped_df2 = measurement_df.groupby(grouping_cols)
 
     # data frame has timepoint specific overrides if grouping by noise
     # parameters and observable parameters in addition to observable,
     # condition and preeq id yields more groups
-    return len(grouped_df.index) != len(grouped_df2.index)
+    return len(grouped_df) != len(grouped_df2)
+
+
+def observable_table_has_nontrivial_noise_formula(
+        observable_df: pd.DataFrame) -> bool:
+    """
+    Does any observable have a noise formula that is not just a single
+    parameter?
+
+    Arguments:
+        observable_df: PEtab observable table
+
+    Returns:
+        True if any noise formula does not consist of a single identifier,
+        False otherwise.
+    """
+
+    return not observable_df[NOISE_FORMULA].apply(
+        lambda x: re.match(r'^[\w_\.]+$', x) is not None
+    ).all()
 
 
 def measurement_table_has_observable_parameter_numeric_overrides(
@@ -598,7 +634,7 @@ def measurement_table_has_observable_parameter_numeric_overrides(
         measurement_df: PEtab measurement table
 
     Returns:
-        True if there any numbers to override observable parameters,
+        True if there are any numbers to override observable/noise parameters,
         False otherwise.
     """
     if OBSERVABLE_PARAMETERS not in measurement_df:

tests/test_lint.py

@@ -57,17 +57,50 @@ def test_measurement_table_has_timepoint_specific_mappings():
         PREEQUILIBRATION_CONDITION_ID: [nan, nan],
         TIME: [1.0, 2.0],
         OBSERVABLE_PARAMETERS: ['obsParOverride', ''],
-        NOISE_PARAMETERS: ['', '']
+        NOISE_PARAMETERS: ['1.0', 1.0]
     })
 
     assert lint.measurement_table_has_timepoint_specific_mappings(
         measurement_df) is True
 
+    # both measurements different anyways
     measurement_df.loc[1, OBSERVABLE_ID] = 'obs2'
+    assert lint.measurement_table_has_timepoint_specific_mappings(
+        measurement_df) is False
 
+    # mixed numeric string
+    measurement_df.loc[1, OBSERVABLE_ID] = 'obs1'
+    measurement_df.loc[1, OBSERVABLE_PARAMETERS] = 'obsParOverride'
     assert lint.measurement_table_has_timepoint_specific_mappings(
         measurement_df) is False
 
+    # different numeric values
+    measurement_df.loc[1, NOISE_PARAMETERS] = 2.0
+    assert lint.measurement_table_has_timepoint_specific_mappings(
+        measurement_df) is True
+    assert lint.measurement_table_has_timepoint_specific_mappings(
+        measurement_df, allow_scalar_numeric_noise_parameters=True) is False
+
+
+def test_observable_table_has_nontrivial_noise_formula():
+    # Ensure we fail if we have nontrivial noise formulas
+
+    observable_df = pd.DataFrame(data={
+        OBSERVABLE_ID: ['0obsPar1noisePar', '2obsPar0noisePar'],
+        OBSERVABLE_FORMULA: ['1.0',
+                             '1.0'],
+        NOISE_FORMULA: ['noiseParameter1_0obsPar1noisePar + 3.0',
+                        '1.0']
+    })
+
+    assert lint.observable_table_has_nontrivial_noise_formula(observable_df)\
+        is True
+
+    observable_df.loc[0, NOISE_FORMULA] = 'sigma1'
+
+    assert lint.observable_table_has_nontrivial_noise_formula(observable_df) \
+        is False
+
 
 def test_assert_overrides_match_parameter_count():
     # Ensure we recognize and fail if we have wrong number of overrides