style: improve repr

Author: glichtner

execution_engine/fhir/util.py

@@ -13,7 +13,9 @@ def get_coding(cc: CodeableConcept, system_uri: str | None = None) -> Coding:
         coding = [c for c in cc.coding if c.system == system_uri]
 
     if cc.coding is None or len(coding) != 1:
-        raise ValueError("CodeableConcept must have exactly one coding")
+        raise ValueError(
+            f"CodeableConcept must have exactly one coding, got {len(coding)}"
+        )
 
     return coding[0]
 

execution_engine/omop/cohort/population_intervention_pair.py

@@ -63,8 +63,8 @@ def __repr__(self) -> str:
         """
         return (
             f"{self.__class__.__name__}(\n"
-            f"  name={self._name},\n"
-            f"  url={self._url},\n"
+            f"  name={repr(self._name)},\n"
+            f"  url={repr(self._url)},\n"
             f"  base_criterion={repr(self._base_criterion)},\n"
             f"  population={self._population._repr_pretty(level=1).strip()},\n"
             f"  intervention={self._intervention._repr_pretty(level=1).strip()}\n"

execution_engine/omop/cohort/recommendation.py

@@ -75,14 +75,13 @@ def __repr__(self) -> str:
         return (
             f"{self.__class__.__name__}(\n"
             f"  pi_pairs={pi_repr},\n"
-            f"  base_criterion={self._base_criterion},\n"
-            f"  name='{self._name}',\n"
-            f"  title='{self._title}',\n"
-            f"  url='{self._url}',\n"
-            f"  version='{self._version}',\n"
-            f"  description='{self._description}',\n"
-            f"  recommendation_id={self._id}\n"
-            f"  package_version='{self._package_version}',\n"
+            f"  base_criterion={repr(self._base_criterion)},\n"
+            f"  name={repr(self._name)},\n"
+            f"  title={repr(self._title)},\n"
+            f"  url={repr(self._url)},\n"
+            f"  version={repr(self._version)},\n"
+            f"  description={repr(self._description)},\n"
+            f"  package_version={repr(self._package_version)},\n"
             f")"
         )
 

execution_engine/omop/criterion/combination/combination.py

@@ -38,9 +38,9 @@ def __repr__(self) -> str:
             Get the string representation of the operator.
             """
             if self.operator in ["AT_LEAST", "AT_MOST", "EXACTLY"]:
-                return f'{self.__class__.__name__}(operator="{self.operator}", threshold={self.threshold})'
+                return f'{self.__class__.__qualname__}(operator="{self.operator}", threshold={self.threshold})'
             else:
-                return f'{self.__class__.__name__}(operator="{self.operator}")'
+                return f'{self.__class__.__qualname__}(operator="{self.operator}")'
 
         def __eq__(self, other: object) -> bool:
             """
@@ -207,6 +207,8 @@ def _build_repr(
         children: Sequence[tuple[str | None, Any]],
         params: list[tuple[str, Any]],
         level: int = 0,
+        children_param_name: str = "criteria",
+        children_is_sequence: bool = True,
     ) -> str:
         """
         Builds a multi-line string for this criterion combination,
@@ -265,11 +267,17 @@ def _build_repr(
         if method_defined:
             lines.extend(criteria_lines)
             lines.extend(kw_lines)
-        else:
+        elif children_is_sequence:
             lines.extend(kw_lines)
-            lines.append(f"{child_indent}criteria=[")
+            lines.append(f"{child_indent}{children_param_name}=[")
             lines.extend(criteria_lines)
             lines.append(f"{child_indent}],")
+        else:
+            assert len(criteria_lines) == 1
+            lines.extend(kw_lines)
+            lines.append(
+                f"{child_indent}{children_param_name}={criteria_lines[0].strip()}"
+            )
 
         lines.append(f"{indent})")
 

execution_engine/omop/criterion/combination/temporal.py

@@ -73,6 +73,17 @@ def __init__(
 
         super().__init__(operator=operator, criteria=criteria)
 
+    def _repr_pretty(self, level: int = 0) -> str:
+        children = [(None, c) for c in self._criteria]
+
+        return self._build_repr(
+            children,
+            params=[],
+            level=level,
+            children_param_name="criterion",
+            children_is_sequence=False,
+        )
+
 
 class FixedWindowTemporalIndicatorCombination(TemporalIndicatorCombination):
     """
@@ -138,7 +149,13 @@ def _repr_pretty(self, level: int = 0) -> str:
             ("start_time", self.start_time),
             ("end_time", self.end_time),
         ]
-        return self._build_repr(children, params, level)
+        return self._build_repr(
+            children,
+            params=params,
+            level=level,
+            children_param_name="criterion",
+            children_is_sequence=False,
+        )
 
     def dict(self) -> Dict:
         """
@@ -377,7 +394,13 @@ def _repr_pretty(self, level: int = 0) -> str:
         params = [
             ("interval_criterion", self.interval_criterion),
         ]
-        return self._build_repr(children, params, level)
+        return self._build_repr(
+            children,
+            params=params,
+            level=level,
+            children_param_name="criterion",
+            children_is_sequence=False,
+        )
 
     @property
     def interval_criterion(self) -> Criterion | CriterionCombination:

execution_engine/omop/criterion/meta.py

@@ -79,7 +79,7 @@ def __init__(self: object, *args: Any, **kwargs: Any) -> None:
         original_repr = namespace.get("__repr__")
 
         if not original_repr:
-            # Try to find an __init__ in the bases
+            # Try to find an __repr__ in the bases
             for base in reversed(bases):
                 if base.__repr__ is not object.__repr__:
                     original_repr = base.__repr__
@@ -91,23 +91,30 @@ def __init__(self: object, *args: Any, **kwargs: Any) -> None:
         ):
 
             def __repr__(self: object) -> str:
+                # Case 1: No real __init__ found at all => just return ClassName()
+                if original_init is None:
+                    return f"{name}()"
+
+                # Case 2: If the class or its parents do define an __init__, we check
+                # for _init_args. If the class isn't wrapping init, your class would
+                # have to set _init_args itself (or you'll just get a normal object repr).
                 if not hasattr(self, "_init_args"):
                     return super(type(self), self).__repr__()
-                sig = inspect.signature(original_init) if original_init else None
+
+                # Build param=value only if they differ from default
+                sig = inspect.signature(original_init)
                 parts = []
-                if sig:
-                    for param_name, param in sig.parameters.items():
-                        if param_name == "self":
-                            continue
-                        default = param.default
-                        # Only show params if they differ from the default
-                        if (
-                            param_name in self._init_args
-                            and self._init_args[param_name] != default
-                        ):
-                            parts.append(
-                                f"{param_name}={repr(self._init_args[param_name])}"
-                            )
+                for param_name, param in sig.parameters.items():
+                    if param_name == "self":
+                        continue
+                    default = param.default
+                    if (
+                        param_name in self._init_args
+                        and self._init_args[param_name] != default
+                    ):
+                        parts.append(
+                            f"{param_name}={repr(self._init_args[param_name])}"
+                        )
                 return f"{name}({', '.join(parts)})"
 
             # Tag it so children know it's auto-generated