Parameter Fitter: Check ranges for manual edit

Author: janezd

Orange/widgets/evaluate/owparameterfitter.py

@@ -9,6 +9,7 @@
 
 import pyqtgraph as pg
 
+from orangewidget.utils.itemmodels import signal_blocking
 from orangewidget.utils.visual_settings_dlg import VisualSettingsDialog, \
     KeyType, ValueType
 
@@ -331,7 +332,10 @@ class Error(OWWidget.Error):
         unknown_err = Msg("{}")
         not_enough_data = Msg(f"At least {N_FOLD} instances are needed.")
         incompatible_learner = Msg("{}")
-        manual_steps_error = Msg("Invalid list of steps for {}:\n{}")
+        manual_steps_error = Msg("Invalid list of values for {}:\n{}")
+        manual_invalid_minmax = Msg("Value for {} must be between {} and {}")
+        manual_invalid_minimum = Msg("Value for {} must be at least {}")
+        manual_invalid_maximum = Msg("Value for {} must be at most {}")
         min_max_error = Msg("Minimum must be less than maximum.")
 
     class Warning(OWWidget.Warning):
@@ -398,14 +402,14 @@ def _add_controls(self):
 
         gui.appendRadioButton(buttons, "Manual:")
         layout.addWidget(buttons, 4, 0)
-        edit = gui.lineEdit(None, self, "manual_steps",
+        self.edit = gui.lineEdit(None, self, "manual_steps",
                             placeholderText="10, 20, 30",
                             callback=self.__on_manual_changed)
-        layout.addWidget(edit, 4, 1)
+        layout.addWidget(self.edit, 4, 1)
 
         # gui.lineEdit's connect does not call the callback on return pressed
         # if the line hasn't changed.
-        @edit.returnPressed.connect
+        @self.edit.returnPressed.connect
         def _():
             if self.type != self.MANUAL:
                 self.type = self.MANUAL
@@ -459,6 +463,10 @@ def initial_parameters(self) -> dict:
     def steps(self) -> tuple[int]:
         self.Error.min_max_error.clear()
         self.Error.manual_steps_error.clear()
+        self.Error.manual_invalid_minimum.clear()
+        self.Error.manual_invalid_maximum.clear()
+        self.Error.manual_invalid_minmax.clear()
+
         if self.type == self.FROM_RANGE:
             return self._steps_from_range()
         else:
@@ -480,20 +488,33 @@ def _steps_from_range(self) -> tuple[int]:
                 *range((self.minimum // step + 1) * step, self.maximum, step),
                 self.maximum)
 
-    def _steps_from_manual(self) -> tuple[int]:
+    def _steps_from_manual(self) -> tuple[int, ...]:
+        param = self.fitted_parameters[self.parameter_index]
         steps = self.manual_steps
         if not steps:
             return ()
         try:
-            steps = tuple(sorted(map(int, steps.split(","))))
-            self.manual_steps = ", ".join(map(str, steps))
-            return steps
+            steps = tuple(sorted(set(map(int, steps.split(",")))))
         except ValueError as exc:
-            self.Error.manual_steps_error(
-                self.fitted_parameters[self.parameter_index].label,
-                exc)
+            self.Error.manual_steps_error(param.name, exc)
+            return ()
+
+        self.manual_steps = ", ".join(map(str, steps))
+
+        under = param.min is not None and steps[0] < param.min
+        over = param.max is not None and steps[-1] > param.max
+        if under or over:
+            if under and over:
+                self.Error.manual_invalid_minmax(param.name, param.min, param.max)
+            elif under:
+                self.Error.manual_invalid_minimum(param.name, param.min)
+            else:
+                assert over
+                self.Error.manual_invalid_maximum(param.name, param.max)
             return ()
 
+        return steps
+
     @Inputs.data
     @check_multiple_targets_input
     def set_data(self, data: Optional[Table]):
@@ -548,22 +569,35 @@ def _set_range_controls(self):
         assert param.type == int, \
             "The widget currently supports only int parameters"
 
+        # Block signals to avoid changing `self.type`
+        with signal_blocking(self.__spin_min), signal_blocking(self.__spin_max):
+            if param.min is not None:
+                self.__spin_min.setMinimum(param.min)
+                self.__spin_max.setMinimum(param.min)
+                self.minimum = param.min
+            else:
+                self.__spin_min.setMinimum(-MIN_MAX_SPIN)
+                self.__spin_max.setMinimum(-MIN_MAX_SPIN)
+                self.minimum = self.initial_parameters[param.name]
+            if param.max is not None:
+                self.__spin_min.setMaximum(param.max)
+                self.__spin_max.setMaximum(param.max)
+                self.maximum = param.max
+            else:
+                self.__spin_min.setMaximum(MIN_MAX_SPIN)
+                self.__spin_max.setMaximum(MIN_MAX_SPIN)
+                self.maximum = self.initial_parameters[param.name]
+
+        tip = f"Enter a list of comma-separated values"
         if param.min is not None:
-            self.__spin_min.setMinimum(param.min)
-            self.__spin_max.setMinimum(param.min)
-            self.minimum = param.min
-        else:
-            self.__spin_min.setMinimum(-MIN_MAX_SPIN)
-            self.__spin_max.setMinimum(-MIN_MAX_SPIN)
-            self.minimum = self.initial_parameters[param.name]
-        if param.max is not None:
-            self.__spin_min.setMaximum(param.max)
-            self.__spin_max.setMaximum(param.max)
-            self.maximum = param.max
+            if param.max is not None:
+                self.edit.setToolTip(f"{tip} between {param.min} and {param.max}.")
+            else:
+                self.edit.setToolTip(f"{tip} greater or equal to {param.min}.")
+        elif param.max is not None:
+            self.edit.setToolTip(f"{tip} smaller or equal to {param.max}.")
         else:
-            self.__spin_min.setMaximum(MIN_MAX_SPIN)
-            self.__spin_max.setMaximum(MIN_MAX_SPIN)
-            self.maximum = self.initial_parameters[param.name]
+            self.edit.setToolTip("")
 
     def _update_preview(self):
         if self.type == self.FROM_RANGE:

Orange/widgets/evaluate/tests/test_owparameterfitter.py

@@ -21,8 +21,8 @@
 class DummyLearner(PLSRegressionLearner):
     def fitted_parameters(self):
         return [
-            self.FittedParameter("n_components", "Foo", "foo", int, 1, None),
-            self.FittedParameter("n_components", "Bar", "bar", int, 1, 10),
+            self.FittedParameter("n_components", "Foo", "foo", int, 5, None),
+            self.FittedParameter("n_components", "Bar", "bar", int, 5, 10),
             self.FittedParameter("n_components", "Baz", "baz", int, None, 10),
             self.FittedParameter("n_components", "Qux", "qux", int, None, None)
         ]
@@ -134,6 +134,82 @@ def test_manual_steps(self):
         x = self.widget.graph._FitterPlot__bar_item_cv.opts["x"]
         self.assertEqual(list(x), [0.2, 1.2, 2.2])
 
+    def test_manual_steps_limits(self):
+        w = self.widget
+
+        def test_errors(act):
+            err = w.Error
+            for error in (err.manual_steps_error, err.manual_invalid_minmax,
+                          err.manual_invalid_minimum, err.manual_invalid_maximum):
+                self.assertIs(error.is_shown(), error is act)
+
+        def enter(text):
+            w.controls.manual_steps.setText(text)
+            w.controls.manual_steps.returnPressed.emit()
+
+        self.send_signal(w.Inputs.data, self._housing)
+        self.send_signal(w.Inputs.learner, self._dummy)
+        self.wait_until_finished()
+
+        # 5 to None
+        simulate.combobox_activate_index(w.controls.parameter_index, 0)
+        self.wait_until_finished()
+
+        enter("6, 9, 7")
+        self.assertEqual(w.steps, (6, 7, 9))
+        test_errors(None)
+
+        enter("6, 9, 7, 3")
+        self.assertEqual(w.steps, ())
+        test_errors(w.Error.manual_invalid_minimum)
+
+        enter("6, 9, 7")
+        self.assertEqual(w.steps, (6, 7, 9))
+        test_errors(None)
+
+        enter("6, 9, 7, 3")
+
+        # None to 10
+        simulate.combobox_activate_index(w.controls.parameter_index, 2)
+        self.wait_until_finished()
+
+        test_errors(None)
+
+        enter("12, 1, 3, -5")
+        self.assertEqual(w.steps, ())
+        test_errors(w.Error.manual_invalid_maximum)
+
+        enter("1, 3, -5")
+        self.assertEqual(w.steps, (-5, 1, 3))
+        test_errors(None)
+
+        enter("12, 1, 3, -5")
+
+        # No limits
+        simulate.combobox_activate_index(w.controls.parameter_index, 3)
+        self.wait_until_finished()
+        self.assertEqual(w.steps, (-5, 1, 3, 12))
+        test_errors(None)
+
+        # 5 to 10
+        simulate.combobox_activate_index(w.controls.parameter_index, 1)
+        self.wait_until_finished()
+
+        self.assertEqual(w.steps, ())
+        test_errors(w.Error.manual_invalid_minmax)
+
+        enter("12, 8, 7, 5")
+        self.assertEqual(w.steps, ())
+        test_errors(w.Error.manual_invalid_maximum)
+
+        enter("8, 7, -5")
+        self.assertEqual(w.steps, ())
+        test_errors(w.Error.manual_invalid_minimum)
+
+        enter("8, 7, 5")
+        self.assertEqual(w.steps, (5, 7, 8))
+        test_errors(None)
+
     def test_steps_preview(self):
         self.send_signal(self.widget.Inputs.data, self._housing)
         self.send_signal(self.widget.Inputs.learner, self._pls)
@@ -353,8 +429,10 @@ def test_steps_from_range_error(self):
 
     def test_steps_from_manual_error(self):
         w: OWParameterFitter = self.widget
-        self.send_signal(w.Inputs.data, self._heart)
+        self.send_signal(w.Inputs.data, self._housing)
         self.send_signal(w.Inputs.learner, self._dummy)
+        self.wait_until_finished()
+        simulate.combobox_activate_index(w.controls.parameter_index, 3)
         w.type = w.MANUAL
 
         w.manual_steps = "1, 2, 3, asdf, 4, 5"
@@ -374,8 +452,10 @@ def test_steps_from_manual_error(self):
 
     def test_steps_from_manual(self):
         w: OWParameterFitter = self.widget
-        self.send_signal(w.Inputs.data, self._heart)
+        self.send_signal(w.Inputs.data, self._housing)
         self.send_signal(w.Inputs.learner, self._dummy)
+        self.wait_until_finished()
+        simulate.combobox_activate_index(w.controls.parameter_index, 3)
         w.type = w.MANUAL
 
         w.manual_steps = "1, 2, 3, 4, 5"
@@ -384,10 +464,26 @@ def test_steps_from_manual(self):
         w.manual_steps = "1, 2, 3, 4, 5, 6"
         self.assertEqual(w.steps, (1, 2, 3, 4, 5, 6))
 
-        # TODO: We may or may not want this.
         w.manual_steps = "1, 2, 10,   3, 4, 123, 5, 6"
         self.assertEqual(w.steps, (1, 2, 3, 4, 5, 6, 10, 123))
 
+    def test_manual_tooltip(self):
+        w: OWParameterFitter = self.widget
+        self.send_signal(w.Inputs.data, self._housing)
+        self.send_signal(w.Inputs.learner, self._dummy)
+        self.wait_until_finished()
+
+        simulate.combobox_activate_index(w.controls.parameter_index, 0)
+        self.assertIn("greater or equal to 5", w.edit.toolTip())
+
+        simulate.combobox_activate_index(w.controls.parameter_index, 1)
+        self.assertIn("between 5 and 10", w.edit.toolTip())
+
+        simulate.combobox_activate_index(w.controls.parameter_index, 2)
+        self.assertIn("smaller or equal to 10", w.edit.toolTip())
+
+        simulate.combobox_activate_index(w.controls.parameter_index, 3)
+        self.assertEqual("", w.edit.toolTip())
 
 if __name__ == "__main__":
     unittest.main()