Misc fixes & annotations (#450)

* type annotations
* fix accessing non-existing model
* remove misleading comments

---------

Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>

Author: dweindl

petab/v1/core.py

@@ -133,15 +133,18 @@ def get_notnull_columns(df: pd.DataFrame, candidates: Iterable):
     ]
 
 
-def get_observable_replacement_id(groupvars, groupvar) -> str:
+def get_observable_replacement_id(
+    groupvars: list[str], groupvar: Sequence
+) -> str:
     """Get the replacement ID for an observable.
 
     Arguments:
         groupvars:
             The columns of a PEtab measurement table that should be unique
             between observables in a flattened PEtab problem.
         groupvar:
-            A specific grouping of `groupvars`.
+            A specific grouping of `groupvars`. Same length and order as
+            `groupvars`.
 
     Returns:
         The observable replacement ID.

petab/v1/measurements.py

@@ -307,7 +307,6 @@ def assert_overrides_match_parameter_count(
                 row.get(OBSERVABLE_PARAMETERS, None)
             )
         )
-        # No overrides are also allowed
         if actual != expected:
             formula = observable_df.loc[row[OBSERVABLE_ID], OBSERVABLE_FORMULA]
             raise AssertionError(
@@ -324,7 +323,6 @@ def assert_overrides_match_parameter_count(
         try:
             expected = noise_parameters_count[row[OBSERVABLE_ID]]
 
-            # No overrides are also allowed
             if len(replacements) != expected:
                 raise AssertionError(
                     f"Mismatch of noise parameter overrides in:\n{row}\n"

petab/v2/core.py

@@ -71,6 +71,8 @@
     "ParameterTable",
 ]
 
+logger = logging.getLogger(__name__)
+
 
 def _is_finite_or_neg_inf(v: float, info: ValidationInfo) -> float:
     if not np.isfinite(v) and v != -np.inf:
@@ -112,7 +114,7 @@ def _valid_petab_id(v: str) -> str:
     return v
 
 
-def _valid_petab_id_or_none(v: str) -> str:
+def _valid_petab_id_or_none(v: str) -> str | None:
     """Field validator for optional PEtab IDs."""
     if not v:
         return None
@@ -252,7 +254,7 @@ def __getitem__(self, id_: str) -> T:
 
     @classmethod
     @abstractmethod
-    def from_df(cls, df: pd.DataFrame) -> BaseTable[T]:
+    def from_df(cls, df: pd.DataFrame, **kwargs) -> BaseTable[T]:
         """Create a table from a DataFrame."""
         pass
 
@@ -1143,7 +1145,11 @@ def __str__(self):
             f"{observables}, {measurements}, {parameters}"
         )
 
-    def __getitem__(self, key):
+    def __getitem__(
+        self, key
+    ) -> (
+        Condition | Experiment | Observable | Measurement | Parameter | Mapping
+    ):
         """Get PEtab entity by ID.
 
         This allows accessing PEtab entities such as conditions, experiments,
@@ -1202,7 +1208,7 @@ def from_yaml(
             from .petab1to2 import petab1to2
 
             if format_version[0] == 1 and yaml_file:
-                logging.debug(
+                logger.debug(
                     "Auto-upgrading problem from PEtab 1.0 to PEtab 2.0"
                 )
                 with TemporaryDirectory() as tmpdirname:
@@ -2320,7 +2326,9 @@ def get_output_parameters(
         # filter out symbols that are defined in the model or mapped to
         #  such symbols
         for candidate in sorted(candidates):
-            if self.model.symbol_allowed_in_observable_formula(candidate):
+            if self.model and self.model.symbol_allowed_in_observable_formula(
+                candidate
+            ):
                 continue
 
             # does it map to a model entity?
@@ -2329,8 +2337,11 @@ def get_output_parameters(
                     mapping.petab_id == candidate
                     and mapping.model_id is not None
                 ):
-                    if self.model.symbol_allowed_in_observable_formula(
-                        mapping.model_id
+                    if (
+                        self.model
+                        and self.model.symbol_allowed_in_observable_formula(
+                            mapping.model_id
+                        )
                     ):
                         break
             else: