Changeset fb560d2 in sasview

Timestamp:

Sep 7, 2018 9:51:27 AM (6 years ago)

Author:

GitHub <noreply@…>

Branches:

ESS_GUI, ESS_GUI_batch_fitting, ESS_GUI_bumps_abstraction, ESS_GUI_iss1116, ESS_GUI_iss879, ESS_GUI_opencl, ESS_GUI_ordering, ESS_GUI_sync_sascalc

Children:

c0de493, c2f3ca2

Parents:

dcabba7 (diff), 4ea8020 (diff)
Note: this is a merge changeset, the changes displayed below correspond to the merge itself.
Use the (diff) links above to see all the changes relative to each parent.

git-author:

Torin Cooper-Bennun <40573959+tcbennun@…> (09/07/18 09:51:27)

git-committer:

GitHub <noreply@…> (09/07/18 09:51:27)

Message:

Merge pull request #167 from SasView?/ESS_GUI_iss966

SASVIEW-966: Organise parameter table by model component

Location:

src/sas/qtgui/Perspectives/Fitting

Files:

• FittingUtilities.py (modified) (6 diffs)
• FittingWidget.py (modified) (20 diffs)
• UnitTesting/FittingWidgetTest.py (modified) (19 diffs)

src/sas/qtgui/Perspectives/Fitting/FittingUtilities.py

@@ -124 +124 @@
     return item
 
-def addSimpleParametersToModel(parameters, is2D):
+def addSimpleParametersToModel(parameters, is2D, parameters_original=None):
     """
     Update local ModelModel with sasmodel parameters
+    parameters_original: list of parameters before any tagging on their IDs, e.g. for product model
+    (so that those are the display names; see below)
     """
     if is2D:
@@ -132 +134 @@
     else:
         params = parameters.iq_parameters
+
+    if parameters_original:
+        # 'parameters_original' contains the parameters as they are to be DISPLAYED, while 'parameters'
+        # contains the parameters as they were renamed; this is for handling name collisions in product model.
+        # The 'real name' of the parameter will be stored in the item's user data.
+        if is2D:
+            params_orig = [p for p in parameters_original.kernel_parameters if p.type != 'magnetic']
+        else:
+            params_orig = parameters_original.iq_parameters
+    else:
+        # no difference in names anyway
+        params_orig = params
+
     item = []
-    for param in params:
+    for param, param_orig in zip(params, params_orig):
         # Create the top level, checkable item
-        item_name = param.name
+        item_name = param_orig.name
         item1 = QtGui.QStandardItem(item_name)
+        item1.setData(param.name, QtCore.Qt.UserRole)
         item1.setCheckable(True)
         item1.setEditable(False)
@@ -144 +160 @@
         item4 = QtGui.QStandardItem(str(param.limits[0]))
         item5 = QtGui.QStandardItem(str(param.limits[1]))
-        item6 = QtGui.QStandardItem(param.units)
+        item6 = QtGui.QStandardItem(str(param.units))
         item6.setEditable(False)
         item.append([item1, item2, item4, item5, item6])
@@ -182 +198 @@
     model.appendRow(item_list)
 
+def addHeadingRowToModel(model, name):
+    """adds a non-interactive top-level row to the model"""
+    header_row = [QtGui.QStandardItem() for i in range(5)]
+    header_row[0].setText(name)
+
+    font = header_row[0].font()
+    font.setBold(True)
+    header_row[0].setFont(font)
+
+    for item in header_row:
+        item.setEditable(False)
+        item.setCheckable(False)
+        item.setSelectable(False)
+
+    model.appendRow(header_row)
+
 def addHeadersToModel(model):
     """
@@ -227 +259 @@
     model.header_tooltips = copy.copy(poly_header_error_tooltips)
 
-def addShellsToModel(parameters, model, index):
-    """
-    Find out multishell parameters and update the model with the requested number of them
+def addShellsToModel(parameters, model, index, row_num=None):
+    """
+    Find out multishell parameters and update the model with the requested number of them.
+    Inserts them after the row at row_num, if not None; otherwise, appends to end.
+    Returns a list of lists of QStandardItem objects.
     """
     multishell_parameters = getIterParams(parameters)
 
+    rows = []
     for i in range(index):
         for par in multishell_parameters:
@@ -259 +294 @@
             item4 = QtGui.QStandardItem(str(par.limits[1]))
             item5 = QtGui.QStandardItem(par.units)
-            model.appendRow([item1, item2, item3, item4, item5])
+            row = [item1, item2, item3, item4, item5]
+            rows.append(row)
+
+            if row_num is None:
+                model.appendRow(row)
+            else:
+                model.insertRow(row_num, row)
+                row_num += 1
+
+    return rows
 
 def calculateChi2(reference_data, current_data):

src/sas/qtgui/Perspectives/Fitting/FittingWidget.py

@@ -219 +219 @@
         # Utility variable to enable unselectable option in category combobox
         self._previous_category_index = 0
-        # Utility variable for multishell display
-        self._last_model_row = 0
+        # Utility variables for multishell display
+        self._n_shells_row = 0
+        self._num_shell_params = 0
         # Dictionary of {model name: model class} for the current category
         self.models = {}
@@ -676 +677 @@
         Return list of all parameters for the current model
         """
-        return [self._model_model.item(row).text() for row in range(self._model_model.rowCount())]
+        return [self._model_model.item(row).text()
+                for row in range(self._model_model.rowCount())
+                if self.isCheckable(row)]
 
     def modifyViewOnRow(self, row, font=None, brush=None):
@@ -704 +707 @@
         assert isinstance(constraint, Constraint)
         assert 0 <= row <= self._model_model.rowCount()
+        assert self.isCheckable(row)
 
         item = QtGui.QStandardItem()
@@ -724 +728 @@
         max_col = self.lstParams.itemDelegate().param_max
         for row in self.selectedParameters():
+            assert(self.isCheckable(row))
             param = self._model_model.item(row, 0).text()
             value = self._model_model.item(row, 1).text()
@@ -766 +771 @@
         max_col = self.lstParams.itemDelegate().param_max
         for row in range(self._model_model.rowCount()):
+            if not self.isCheckable(row):
+                continue
             if not self.rowHasConstraint(row):
                 continue
@@ -794 +801 @@
         For the given row, return its constraint, if any
         """
-        try:
+        if self.isCheckable(row):
             item = self._model_model.item(row, 1)
-            return item.child(0).data()
-        except AttributeError:
-            # return none when no constraints
-            return None
+            try:
+                return item.child(0).data()
+            except AttributeError:
+                # return none when no constraints
+                pass
+        return None
 
     def rowHasConstraint(self, row):
@@ -805 +814 @@
         Finds out if row of the main model has a constraint child
         """
-        item = self._model_model.item(row, 1)
-        if item.hasChildren():
-            c = item.child(0).data()
-            if isinstance(c, Constraint):
-                return True
+        if self.isCheckable(row):
+            item = self._model_model.item(row, 1)
+            if item.hasChildren():
+                c = item.child(0).data()
+                if isinstance(c, Constraint):
+                    return True
         return False
 
@@ -816 +826 @@
         Finds out if row of the main model has an active constraint child
         """
-        item = self._model_model.item(row, 1)
-        if item.hasChildren():
-            c = item.child(0).data()
-            if isinstance(c, Constraint) and c.active:
-                return True
+        if self.isCheckable(row):
+            item = self._model_model.item(row, 1)
+            if item.hasChildren():
+                c = item.child(0).data()
+                if isinstance(c, Constraint) and c.active:
+                    return True
         return False
 
@@ -827 +838 @@
         Finds out if row of the main model has an active, nontrivial constraint child
         """
-        item = self._model_model.item(row, 1)
-        if item.hasChildren():
-            c = item.child(0).data()
-            if isinstance(c, Constraint) and c.func and c.active:
-                return True
+        if self.isCheckable(row):
+            item = self._model_model.item(row, 1)
+            if item.hasChildren():
+                c = item.child(0).data()
+                if isinstance(c, Constraint) and c.func and c.active:
+                    return True
         return False
 
@@ -1577 +1589 @@
             # internal so can use closure for param_dict
             param_name = str(self._model_model.item(row, 0).text())
-            if param_name not in list(param_dict.keys()):
+            if not self.isCheckable(row) or param_name not in list(param_dict.keys()):
                 return
             # modify the param value
@@ -1589 +1601 @@
             # Utility function for updateof polydispersity part of the main model
             param_name = str(self._model_model.item(row, 0).text())+'.width'
-            if param_name not in list(param_dict.keys()):
+            if not self.isCheckable(row) or param_name not in list(param_dict.keys()):
                 return
             # modify the param value
@@ -1966 +1978 @@
         # Crete/overwrite model items
         self._model_model.clear()
-
-        # First, add parameters from the main model
-        if model_name is not None:
+        self._poly_model.clear()
+        self._magnet_model.clear()
+
+        if model_name is None:
+            if structure_factor not in (None, "None"):
+                # S(Q) on its own, treat the same as a form factor
+                self.kernel_module = None
+                self.fromStructureFactorToQModel(structure_factor)
+            else:
+                # No models selected
+                return
+        else:
             self.fromModelToQModel(model_name)
-
-        # Then, add structure factor derived parameters
-        if structure_factor is not None and structure_factor != "None":
-            if model_name is None:
-                # Instantiate the current sasmodel for SF-only models
-                self.kernel_module = self.models[structure_factor]()
-            self.fromStructureFactorToQModel(structure_factor)
-        else:
+            self.addExtraShells()
+
             # Allow the SF combobox visibility for the given sasmodel
             self.enableStructureFactorControl(structure_factor)
+        
+            # Add S(Q)
             if self.cbStructureFactor.isEnabled():
                 structure_factor = self.cbStructureFactor.currentText()
                 self.fromStructureFactorToQModel(structure_factor)
 
-        # Then, add multishells
-        if model_name is not None:
-            # Multishell models need additional treatment
-            self.addExtraShells()
-
-        # Add polydispersity to the model
-        self.poly_params = {}
-        self.setPolyModel()
-        # Add magnetic parameters to the model
-        self.magnet_params = {}
-        self.setMagneticModel()
+            # Add polydispersity to the model
+            self.poly_params = {}
+            self.setPolyModel()
+            # Add magnetic parameters to the model
+            self.magnet_params = {}
+            self.setMagneticModel()
 
         # Adjust the table cells width
@@ -2067 +2079 @@
         self.shell_names = self.shellNamesList()
 
-        # Update the QModel
+        # Get new rows for QModel
         new_rows = FittingUtilities.addParametersToModel(self.model_parameters, self.kernel_module, self.is2D)
 
+        # Add heading row
+        FittingUtilities.addHeadingRowToModel(self._model_model, model_name)
+
+        # Update QModel
         for row in new_rows:
             self._model_model.appendRow(row)
-        # Update the counter used for multishell display
-        self._last_model_row = self._model_model.rowCount()
 
     def fromStructureFactorToQModel(self, structure_factor):
@@ -2081 +2095 @@
         if structure_factor is None or structure_factor=="None":
             return
-        structure_module = generate.load_kernel_module(structure_factor)
-        structure_parameters = modelinfo.make_parameter_table(getattr(structure_module, 'parameters', []))
-
-        structure_kernel = self.models[structure_factor]()
-        form_kernel = self.kernel_module
-
-        self.kernel_module = MultiplicationModel(form_kernel, structure_kernel)
-
-        new_rows = FittingUtilities.addSimpleParametersToModel(structure_parameters, self.is2D)
+
+        s_kernel = self.models[structure_factor]()
+        p_kernel = self.kernel_module
+
+        if p_kernel is None:
+            # Not a product model, just S(Q)
+            self.kernel_module = s_kernel
+            params = modelinfo.ParameterTable(self.kernel_module._model_info.parameters.kernel_parameters)
+            new_rows = FittingUtilities.addSimpleParametersToModel(params, self.is2D)
+        else:
+            p_pars_len = len(p_kernel._model_info.parameters.kernel_parameters)
+            s_pars_len = len(s_kernel._model_info.parameters.kernel_parameters)
+
+            self.kernel_module = MultiplicationModel(p_kernel, s_kernel)
+            all_params = self.kernel_module._model_info.parameters.kernel_parameters
+            all_param_names = [param.name for param in all_params]
+
+            # S(Q) params from the product model are not necessarily the same as those from the S(Q) model; any
+            # conflicting names with P(Q) params will cause a rename; we also lose radius_effective (for now...)
+
+            # TODO: merge rest of beta approx implementation in
+            # This is to ensure compatibility when we merge beta approx support in...!
+
+            # radius_effective is always s_params[0]
+
+            # if radius_effective_mode is in all_params, then all_params contains radius_effective and we want to
+            # keep it in the model
+
+            # if radius_effective_mode is NOT in all_params, then radius_effective should NOT be kept, because the user
+            # cannot specify it themselves; but, make sure we only remove it if it's actually there in the first place
+            # (sasmodels master removes it already)
+            if "radius_effective_mode" in all_param_names:
+                # Show all parameters
+                s_params = modelinfo.ParameterTable(all_params[p_pars_len:p_pars_len+s_pars_len])
+                s_params_orig = modelinfo.ParameterTable(s_kernel._model_info.parameters.kernel_parameters)
+            else:
+                # Ensure radius_effective is not displayed
+                s_params_orig = modelinfo.ParameterTable(s_kernel._model_info.parameters.kernel_parameters[1:])
+                if "radius_effective" in all_param_names:
+                    s_params = modelinfo.ParameterTable(all_params[p_pars_len+1:p_pars_len+s_pars_len])
+                else:
+                    s_params = modelinfo.ParameterTable(all_params[p_pars_len:p_pars_len+s_pars_len-1])
+
+            # Get new rows for QModel
+            # Any renamed parameters are stored as data in the relevant item, for later handling
+            new_rows = FittingUtilities.addSimpleParametersToModel(s_params, self.is2D, s_params_orig)
+
+            # TODO: merge rest of beta approx implementation in
+            # These parameters are not part of P(Q) nor S(Q), but are added only to the product model (e.g. specifying
+            # structure factor calculation mode)
+            # product_params = all_params[p_pars_len+s_pars_len:]
+
+        # Add heading row
+        FittingUtilities.addHeadingRowToModel(self._model_model, structure_factor)
+
+        # Update QModel
         for row in new_rows:
             self._model_model.appendRow(row)
             # disable fitting of parameters not listed in self.kernel_module (probably radius_effective)
-            if row[0].text() not in self.kernel_module.params.keys():
-                row_num = self._model_model.rowCount() - 1
-                FittingUtilities.markParameterDisabled(self._model_model, row_num)
-
-        # Update the counter used for multishell display
-        self._last_model_row = self._model_model.rowCount()
+            # if row[0].text() not in self.kernel_module.params.keys():
+            #     row_num = self._model_model.rowCount() - 1
+            #     FittingUtilities.markParameterDisabled(self._model_model, row_num)
 
     def haveParamsToFit(self):
@@ -2124 +2182 @@
         model_row = item.row()
         name_index = self._model_model.index(model_row, 0)
+        name_item = self._model_model.itemFromIndex(name_index)
 
         # Extract changed value.
@@ -2132 +2191 @@
             return
 
-        parameter_name = str(self._model_model.data(name_index)) # sld, background etc.
+        # if the item has user data, this is the actual parameter name (e.g. to handle duplicate names)
+        if name_item.data(QtCore.Qt.UserRole):
+            parameter_name = str(name_item.data(QtCore.Qt.UserRole))
+        else:
+            parameter_name = str(self._model_model.data(name_index))
 
         # Update the parameter value - note: this supports +/-inf as well
@@ -2366 +2429 @@
             new_plots.append(sq_data)
 
-        # Update/generate plots
         for plot in new_plots:
             self.communicate.plotUpdateSignal.emit([plot])
@@ -2566 +2628 @@
         def updateFunctionCaption(row):
             # Utility function for update of polydispersity function name in the main model
+            if not self.isCheckable(row):
+                return
             self._model_model.blockSignals(True)
             param_name = str(self._model_model.item(row, 0).text())
@@ -2738 +2802 @@
 
         self.lstParams.setIndexWidget(shell_index, func)
-        self._last_model_row = self._model_model.rowCount()
+        self._n_shells_row = shell_row - 1
 
         # Set the index to the state-kept value
@@ -2749 +2813 @@
         """
         # Find row location of the combobox
-        last_row = self._last_model_row
-        remove_rows = self._model_model.rowCount() - last_row
+        first_row = self._n_shells_row + 1
+        remove_rows = self._num_shell_params
 
         if remove_rows > 1:
-            self._model_model.removeRows(last_row, remove_rows)
-
-        FittingUtilities.addShellsToModel(self.model_parameters, self._model_model, index)
+            self._model_model.removeRows(first_row, remove_rows)
+
+        new_rows = FittingUtilities.addShellsToModel(self.model_parameters, self._model_model, index, first_row)
+        self._num_shell_params = len(new_rows)
+
         self.current_shell_displayed = index
 

src/sas/qtgui/Perspectives/Fitting/UnitTesting/FittingWidgetTest.py

@@ -256 +256 @@
         self.widget.cbStructureFactor.setCurrentIndex(structure_index)
 
-        # We have 4 more rows now
+        # We have 3 more param rows now (radius_effective is removed), and a new heading
         self.assertEqual(self.widget._model_model.rowCount(), rowcount+4)
 
@@ -276 +276 @@
         last_index = self.widget.cbStructureFactor.count()
         self.widget.cbStructureFactor.setCurrentIndex(last_index-1)
-        # Do we have all the rows?
-        self.assertEqual(self.widget._model_model.rowCount(), 4)
+        # Do we have all the rows (incl. radius_effective & heading row)?
+        self.assertEqual(self.widget._model_model.rowCount(), 5)
 
         # Are the command buttons properly enabled?
@@ -509 +509 @@
         Test opening of the load file dialog for 'array' polydisp. function
         """
+
+        # open a non-existent file
         filename = os.path.join("UnitTesting", "testdata_noexist.txt")
+        with self.assertRaises(OSError, msg="testdata_noexist.txt should be a non-existent file"):
+            os.stat(filename)
         QtWidgets.QFileDialog.getOpenFileName = MagicMock(return_value=(filename,''))
         self.widget.show()
@@ -525 +529 @@
 
         # good file
+        # TODO: this depends on the working directory being src/sas/qtgui,
+        # TODO: which isn't convenient if you want to run this test suite
+        # TODO: individually
         filename = os.path.join("UnitTesting", "testdata.txt")
+        try:
+            os.stat(filename)
+        except OSError:
+            self.assertTrue(False, "testdata.txt does not exist")
         QtWidgets.QFileDialog.getOpenFileName = MagicMock(return_value=(filename,''))
 
@@ -591 +602 @@
 
         # Assure we have the combobox available
-        last_row = self.widget._last_model_row
-        func_index = self.widget._model_model.index(last_row-1, 1)
+        cbox_row = self.widget._n_shells_row
+        func_index = self.widget._model_model.index(cbox_row, 1)
         self.assertIsInstance(self.widget.lstParams.indexWidget(func_index), QtWidgets.QComboBox)
+
+        # get number of rows before changing shell count
+        last_row = self.widget._model_model.rowCount()
 
         # Change the combo box index
@@ -1027 +1041 @@
 
          # Check the model
-        self.assertEqual(self.widget._model_model.rowCount(), 6)
+        self.assertEqual(self.widget._model_model.rowCount(), 7)
         self.assertEqual(self.widget._model_model.columnCount(), 5)
 
@@ -1143 +1157 @@
         # two rows selected
         index1 = self.widget.lstParams.model().index(1, 0, QtCore.QModelIndex())
-        index2 = self.widget.lstParams.model().index(2, 0, QtCore.QModelIndex())
+        index2 = self.widget.lstParams.model().index(3, 0, QtCore.QModelIndex())
         selection_model = self.widget.lstParams.selectionModel()
         selection_model.select(index1, selection_model.Select | selection_model.Rows)
@@ -1179 +1193 @@
         # several random parameters
         self.assertEqual(self.widget.getRowFromName('scale'), 0)
-        self.assertEqual(self.widget.getRowFromName('length'), 5)
+        self.assertEqual(self.widget.getRowFromName('length'), 6)
 
     def testGetParamNames(self):
@@ -1216 +1230 @@
         # Create a constraint object
         const = Constraint(parent=None, value=7.0)
-        row = 2
+        row = 3
 
         spy = QtSignalSpy(self.widget, self.widget.constraintAddedSignal)
@@ -1235 +1249 @@
         # assign complex constraint now
         const = Constraint(parent=None, param='radius', func='5*sld')
-        row = 4
+        row = 5
         # call the method tested
         self.widget.addConstraintToRow(constraint=const, row=row)
@@ -1294 +1308 @@
         self.widget.cbModel.setCurrentIndex(model_index)
 
+        row1 = 1
+        row2 = 5
+
+        param1 = "background"
+        param2 = "radius"
+
+        #default_value1 = "0.001"
+        default_value2 = "20"
+
         # select two rows
-        row1 = 1
-        row2 = 4
         index1 = self.widget.lstParams.model().index(row1, 0, QtCore.QModelIndex())
         index2 = self.widget.lstParams.model().index(row2, 0, QtCore.QModelIndex())
@@ -1313 +1334 @@
 
         # delete one of the constraints
-        self.widget.deleteConstraintOnParameter(param='background')
+        self.widget.deleteConstraintOnParameter(param=param1)
 
         # see that the other constraint is still present
-        cons = self.widget.getConstraintForRow(4) # 4 = radius
-        self.assertEqual(cons.param, "radius")
-        self.assertEqual(cons.value, "20")
+        cons = self.widget.getConstraintForRow(row2)
+        self.assertEqual(cons.param, param2)
+        self.assertEqual(cons.value, default_value2)
 
         # kill the other constraint
@@ -1324 +1345 @@
 
         # see that the other constraint is still present
-        self.assertEqual(self.widget.getConstraintsForModel(), [('radius', None)])
+        self.assertEqual(self.widget.getConstraintsForModel(), [(param2, None)])
 
     def testGetConstraintForRow(self):
@@ -1344 +1365 @@
         self.widget.cbModel.setCurrentIndex(model_index)
 
+        row1 = 1
+        row2 = 5
+
         # select two rows
-        row1 = 1
-        row2 = 4
         index1 = self.widget.lstParams.model().index(row1, 0, QtCore.QModelIndex())
         index2 = self.widget.lstParams.model().index(row2, 0, QtCore.QModelIndex())
@@ -1356 +1378 @@
         self.widget.addSimpleConstraint()
 
-        con_list = [False, True, False, False, True, False]
+        con_list = [False, True, False, False, False, True, False]
         new_list = []
         for row in range(self.widget._model_model.rowCount()):
@@ -1374 +1396 @@
         self.widget.cbModel.setCurrentIndex(model_index)
 
+        row1 = 1
+        row2 = 5
+
         # select two rows
-        row1 = 1
-        row2 = 4
         index1 = self.widget.lstParams.model().index(row1, 0, QtCore.QModelIndex())
         index2 = self.widget.lstParams.model().index(row2, 0, QtCore.QModelIndex())
@@ -1390 +1413 @@
         constraint_objects[0].active = False
 
-        con_list = [False, False, False, False, True, False]
+        con_list = [False, False, False, False, False, True, False]
         new_list = []
         for row in range(self.widget._model_model.rowCount()):
@@ -1411 +1434 @@
         self.assertEqual(self.widget.getConstraintsForModel(),[])
 
+        row1 = 1
+        row2 = 5
+
+        param1 = "background"
+        param2 = "radius"
+
+        default_value1 = "0.001"
+        default_value2 = "20"
+
         # select two rows
-        row1 = 1
-        row2 = 4
         index1 = self.widget.lstParams.model().index(row1, 0, QtCore.QModelIndex())
         index2 = self.widget.lstParams.model().index(row2, 0, QtCore.QModelIndex())
@@ -1425 +1455 @@
         # simple constraints
         # self.assertEqual(self.widget.getConstraintsForModel(), [('background', '0.001'), ('radius', '20')])
-        cons = self.widget.getConstraintForRow(1) # 1 - background
-        self.assertEqual(cons.param, "background")
-        self.assertEqual(cons.value, "0.001")
-        cons = self.widget.getConstraintForRow(4) # 4 = radius
-        self.assertEqual(cons.param, "radius")
-        self.assertEqual(cons.value, "20")
+        cons = self.widget.getConstraintForRow(row1)
+        self.assertEqual(cons.param, param1)
+        self.assertEqual(cons.value, default_value1)
+        cons = self.widget.getConstraintForRow(row2)
+        self.assertEqual(cons.param, param2)
+        self.assertEqual(cons.value, default_value2)
 
         objects = self.widget.getConstraintObjectsForModel()
         self.assertEqual(len(objects), 2)
-        self.assertEqual(objects[1].value, '20')
-        self.assertEqual(objects[0].param, 'background')
+        self.assertEqual(objects[1].value, default_value2)
+        self.assertEqual(objects[0].param, param1)
+
+        row = 0
+        param = "scale"
+        func = "5*sld"
 
         # add complex constraint
-        const = Constraint(parent=None, param='scale', func='5*sld')
-        row = 0
+        const = Constraint(parent=None, param=param, func=func)
         self.widget.addConstraintToRow(constraint=const, row=row)
         #self.assertEqual(self.widget.getConstraintsForModel(),[('scale', '5*sld'), ('background', '0.001'), ('radius', None)])
-        cons = self.widget.getConstraintForRow(4) # 4 = radius
-        self.assertEqual(cons.param, "radius")
-        self.assertEqual(cons.value, "20")
+        cons = self.widget.getConstraintForRow(row2)
+        self.assertEqual(cons.param, param2)
+        self.assertEqual(cons.value, default_value2)
 
         objects = self.widget.getConstraintObjectsForModel()
         self.assertEqual(len(objects), 3)
-        self.assertEqual(objects[0].func, '5*sld')
+        self.assertEqual(objects[0].func, func)
 
     def testReplaceConstraintName(self):