Address comments (#230)

Addresses comments on [SlicerOpenLIFU PR#250
review](#250 (review))

Author: peterhollender

src/openlifu/plan/param_constraint.py

@@ -3,8 +3,8 @@
 from dataclasses import dataclass
 from typing import Annotated, Literal, Tuple
 
-from openlifu.util.dict_conversion import DictMixin
 from openlifu.util.annotations import OpenLIFUFieldData
+from openlifu.util.dict_conversion import DictMixin
 
 PARAM_STATUS_SYMBOLS = {
     "ok": "✅",
@@ -27,13 +27,10 @@ def __post_init__(self):
         if self.warning_value is None and self.error_value is None:
             raise ValueError("At least one of warning_value or error_value must be set")
         if self.operator in ['within','inside','outside','outside_inclusive']:
-            if self.warning_value is not None:
-                if not isinstance(self.warning_value, tuple) or len(self.warning_value) != 2 or (self.warning_value[0] >= self.warning_value[1]):
-                    raise ValueError("Warning value must be a sorted tuple of two numbers")
-
-            if self.error_value is not None:
-                if not isinstance(self.error_value, tuple) or len(self.error_value) != 2 or not (self.error_value[0] >= self.error_value[1]):
-                    raise ValueError("Error value must be a sorted tuple of two numbers")
+            if self.warning_value and (not isinstance(self.warning_value, tuple) or len(self.warning_value) != 2 or (self.warning_value[0] >= self.warning_value[1])):
+                raise ValueError("Warning value must be a sorted tuple of two numbers")
+            if self.error_value and (not isinstance(self.error_value, tuple) or len(self.error_value) != 2 or not (self.error_value[0] >= self.error_value[1])):
+                raise ValueError("Error value must be a sorted tuple of two numbers")
         elif self.operator in ['<','<=','>','>=']:
             if self.warning_value is not None and not isinstance(self.warning_value, (int, float)):
                 raise ValueError("Warning value must be a single value")

src/openlifu/plan/solution.py

@@ -99,15 +99,19 @@ def num_foci(self) -> int:
     def analyze(self,
                 transducer: Transducer,
                 options: SolutionAnalysisOptions = SolutionAnalysisOptions(),
-                param_constraints: Dict[str,ParameterConstraint] = {}) -> SolutionAnalysis:
+                param_constraints: Dict[str,ParameterConstraint]|None = None) -> SolutionAnalysis:
         """Analyzes the treatment solution.
 
         Args:
             transducer: A Transducer item.
             options: A struct for solution analysis options.
+            param_constraints: A dictionary of parameter constraints to apply to the analysis.
+                The keys are the parameter names and the values are the ParameterConstraint objects.
 
         Returns: A struct containing the results of the analysis.
         """
+        if param_constraints is None:
+            param_constraints = {}
         solution_analysis = SolutionAnalysis()
 
         if transducer.id != self.transducer_id:

src/openlifu/plan/solution_analysis.py

@@ -8,6 +8,7 @@
 import pandas as pd
 import xarray as xa
 
+from openlifu.plan.param_constraint import PARAM_STATUS_SYMBOLS, ParameterConstraint
 from openlifu.util.annotations import OpenLIFUFieldData
 from openlifu.util.dict_conversion import DictMixin
 
@@ -109,6 +110,8 @@ def to_table(self, constraints:Dict[str,ParameterConstraint]|None=None, focus_in
             elif fmt[0] == "mean":
                 value_by_focus = self.__dict__[param]
                 agg_value = np.mean(value_by_focus)
+            else:
+                raise ValueError(f"Unknown aggregation method '{fmt[0]}' for parameter '{param}'.")
             if agg_value is not None:
                 record = {"id": param,
                           "Param": fmt[3],

src/openlifu/xdc/element.py

@@ -224,7 +224,4 @@ def to_dict(self):
 
     @staticmethod
     def from_dict(d):
-        if isinstance(d, dict):
-            return [Element(**d)]
-        else:
-            return [Element(**di) for di in d]
+        return Element(**d)

src/openlifu/xdc/transducer.py

@@ -4,7 +4,7 @@
 import json
 import logging
 from dataclasses import dataclass, field
-from typing import Annotated, Any, Dict, List, Tuple
+from typing import Annotated, Any, Dict, List
 
 import numpy as np
 import vtk
@@ -23,7 +23,7 @@ class Transducer:
     name: Annotated[str, OpenLIFUFieldData("Transducer name", "Human readable name for transducer")] = ""
     """Human readable name for transducer"""
 
-    elements: Annotated[Tuple[Element], OpenLIFUFieldData("Elements", "Collection of transducer Elements")] = ()
+    elements: Annotated[List[Element], OpenLIFUFieldData("Elements", "Collection of transducer Elements")] = field(default_factory=list)
     """Collection of transducer Elements"""
 
     frequency: Annotated[float, OpenLIFUFieldData("Frequency (Hz)", "Nominal array frequency (Hz)")] = 400.6e3
@@ -230,7 +230,7 @@ def from_file(filename):
     @staticmethod
     def from_dict(d, **kwargs):
         d = d.copy()
-        d["elements"] = Element.from_dict(d["elements"])
+        d["elements"] = [Element.from_dict(element) for element in d["elements"]]
         if "standoff_transform" in d:
             d["standoff_transform"] = np.array(d["standoff_transform"])
         return Transducer(**d, **kwargs)