OWPythonScript: Explicit is better than implicit

Orange/canvas/scheme/widgetsscheme.py

@@ -820,10 +820,7 @@ def process_signals_for_widget(self, node, widget, signals):
 
             app.setOverrideCursor(Qt.WaitCursor)
             try:
-                if "signal" in inspect.signature(handler).parameters:
-                    handler(*args, signal=link.sink_channel)
-                else:
-                    handler(*args)
+                handler(*args)
             except Exception:
                 sys.excepthook(*sys.exc_info())
                 log.exception("Error calling '%s' of '%s'",

Orange/widgets/data/owpythonscript.py

@@ -352,50 +352,29 @@ def select_row(view, row):
                     QItemSelectionModel.ClearAndSelect)
 
 
-def _create_signals(handler, inputs, outputs):
-    class Inputs:
-        pass
-
-    class Outputs:
-        pass
-
-    for name, type_ in inputs:
-        inp = Input("in_" + name, type_,
-                    default=type_ is not object, multiple=True)
-        setattr(Inputs, "in_" + name, inp)
-        inp(handler)
-
-    for name, type_ in outputs:
-        out = Output("out_" + name, type_)
-        setattr(Outputs, "out_" + name, out)
-
-    return Inputs, Outputs
-
-
 class OWPythonScript(widget.OWWidget):
     name = "Python Script"
     description = "Write a Python script and run it on input data or models."
     icon = "icons/PythonScript.svg"
     priority = 3150
 
-    input_signals = (("data", Table), ("learner", Learner),
-                     ("classifier", Model), ("object", object))
-    input_names = ["in_" + name for name, _ in input_signals]
-
-    output_signals = input_signals
-    output_names = ["out_" + name for name, _ in output_signals]
+    class Inputs:
+        data = Input("Data", Table, replaces="in_data",
+                     default=True, multiple=True)
+        learner = Input("Learner", Learner, replaces="in_learner",
+                        default=True, multiple=True)
+        classifier = Input("Classifier", Model, replaces="in_classifier",
+                           default=True, multiple=True)
+        object = Input("Object", object, replaces="in_object",
+                       default=False, multiple=True)
 
-    def signal_handler(self, obj, id=None, signal=None):
-        id = id[0]
-        dic = getattr(self, signal)
-        if obj is None:
-            if id in dic.keys():
-                del dic[id]
-        else:
-            dic[id] = obj
+    class Outputs:
+        data = Output("Data", Table, replaces="out_data")
+        learner = Output("Learner", Learner, replaces="out_learner")
+        classifier = Output("Classifier", Model, replaces="out_classifier")
+        object = Output("Object", object, replaces="out_object")
 
-    Inputs, Outputs = _create_signals(signal_handler,
-                                      input_signals, output_signals)
+    signal_names = ("data", "learner", "classifier", "object")
 
     libraryListSource = \
         Setting([Script("Hello world", "print('Hello world')\n")])
@@ -409,7 +388,7 @@ class Error(OWWidget.Error):
     def __init__(self):
         super().__init__()
 
-        for name in self.input_names:
+        for name in self.signal_names:
             setattr(self, name, {})
 
         for s in self.libraryListSource:
@@ -421,9 +400,9 @@ def __init__(self):
         gui.label(
             self.infoBox, self,
             "<p>Execute python script.</p><p>Input variables:<ul><li> " +
-            "<li>".join(name + ', ' + name + 's' for name in self.input_names) +
+            "<li>".join(map("in_{0}, in_{0}s".format, self.signal_names)) +
             "</ul></p><p>Output variables:<ul><li>" +
-            "<li>".join(name for name in self.output_names) +
+            "<li>".join(map("out_{0}".format, self.signal_names)) +
             "</ul></p>"
         )
 
@@ -532,6 +511,31 @@ def __init__(self):
         self.controlArea.layout().addStretch(1)
         self.resize(800, 600)
 
+    def handle_input(self, obj, id, signal):
+        id = id[0]
+        dic = getattr(self, signal)
+        if obj is None:
+            if id in dic.keys():
+                del dic[id]
+        else:
+            dic[id] = obj
+
+    @Inputs.data
+    def set_data(self, data, id):
+        self.handle_input(data, id, "data")
+
+    @Inputs.learner
+    def set_learner(self, data, id):
+        self.handle_input(data, id, "learner")
+
+    @Inputs.classifier
+    def set_classifier(self, data, id):
+        self.handle_input(data, id, "classifier")
+
+    @Inputs.object
+    def set_object(self, data, id):
+        self.handle_input(data, id, "object")
+
     def handleNewSignals(self):
         self.unconditional_commit()
 
@@ -652,12 +656,12 @@ def saveScript(self):
 
     def initial_locals_state(self):
         d = {}
-        for name in self.input_names:
+        for name in self.signal_names:
             value = getattr(self, name)
             all_values = list(value.values())
             one_value = all_values[0] if len(all_values) == 1 else None
-            d[name + 's'] = all_values
-            d[name] = one_value
+            d["in_" + name + "s"] = all_values
+            d["in_" + name] = one_value
         return d
 
     def commit(self):
@@ -669,8 +673,8 @@ def commit(self):
         self.console.write("\nRunning script:\n")
         self.console.push("exec(_script)")
         self.console.new_prompt(sys.ps1)
-        for signal in self.output_names:
-            out_var = self.console.locals.get(signal, None)
+        for signal in self.signal_names:
+            out_var = self.console.locals.get("out_" + signal, None)
             signal_type = getattr(self.Outputs, signal).type
             if not isinstance(out_var, signal_type) and out_var is not None:
                 self.Error.add_message(signal,

Orange/widgets/data/tests/test_owpythonscript.py

@@ -16,30 +16,30 @@ def setUp(self):
 
     def test_inputs(self):
         """Check widget's inputs"""
-        for _input, data in (("in_data", self.iris),
-                             ("in_learner", self.learner),
-                             ("in_classifier", self.model),
-                             ("in_object", "object")):
-            self.assertEqual(getattr(self.widget, _input), {})
+        for _input, data in (("Data", self.iris),
+                             ("Learner", self.learner),
+                             ("Classifier", self.model),
+                             ("Object", "object")):
+            self.assertEqual(getattr(self.widget, _input.lower()), {})
             self.send_signal(_input, data, (1,))
-            self.assertEqual(getattr(self.widget, _input), {1: data})
+            self.assertEqual(getattr(self.widget, _input.lower()), {1: data})
             self.send_signal(_input, None, (1,))
-            self.assertEqual(getattr(self.widget, _input), {})
+            self.assertEqual(getattr(self.widget, _input.lower()), {})
 
     def test_outputs(self):
         """Check widget's outputs"""
-        for _input, _output, data in (
-                ("in_data", "out_data", self.iris),
-                ("in_learner", "out_learner", self.learner),
-                ("in_classifier", "out_classifier", self.model),
-                ("in_object", "out_object", "object")):
-            self.widget.text.setPlainText("{} = {}".format(_output, _input))
-            self.send_signal(_input, data, (1,))
-            self.assertIs(self.get_output(_output), data)
-            self.send_signal(_input, None, (1,))
-            self.widget.text.setPlainText("print({})".format(_output))
+        for signal, data in (
+                ("Data", self.iris),
+                ("Learner", self.learner),
+                ("Classifier", self.model)):
+            lsignal = signal.lower()
+            self.widget.text.setPlainText("out_{0} = in_{0}".format(lsignal))
+            self.send_signal(signal, data, (1,))
+            self.assertIs(self.get_output(signal), data)
+            self.send_signal(signal, None, (1,))
+            self.widget.text.setPlainText("print(in_{})".format(lsignal))
             self.widget.execute_button.button.click()
-            self.assertIsNone(self.get_output(_output))
+            self.assertIsNone(self.get_output(signal))
 
     def test_local_variable(self):
         """Check if variable remains in locals after removed from script"""
@@ -59,21 +59,22 @@ def test_wrong_outputs(self):
         """
         self.widget.execute_button.checkbox.setCheckState(False)
         self.assertEqual(len(self.widget.Error.active), 0)
-        for _input, _output, data in (
-                ("in_data", "out_data", self.iris),
-                ("in_learner", "out_learner", self.learner),
-                ("in_classifier", "out_classifier", self.model)):
-            self.send_signal(_input, data, (1, ))
-            self.widget.text.setPlainText("{} = 42".format(_output))
+        for signal, data in (
+                ("Data", self.iris),
+                ("Learner", self.learner),
+                ("Classifier", self.model)):
+            lsignal = signal.lower()
+            self.send_signal(signal, data, (1, ))
+            self.widget.text.setPlainText("out_{} = 42".format(lsignal))
             self.widget.execute_button.button.click()
-            self.assertEqual(self.get_output(_output), None)
-            self.assertTrue(hasattr(self.widget.Error, _output))
-            self.assertTrue(getattr(self.widget.Error, _output).is_shown())
+            self.assertEqual(self.get_output(lsignal), None)
+            self.assertTrue(hasattr(self.widget.Error, lsignal))
+            self.assertTrue(getattr(self.widget.Error, lsignal).is_shown())
 
-            self.widget.text.setPlainText("{} = {}".format(_output, _input))
+            self.widget.text.setPlainText("out_{0} = in_{0}".format(lsignal))
             self.widget.execute_button.button.click()
-            self.assertIs(self.get_output(_output), data)
-            self.assertFalse(getattr(self.widget.Error, _output).is_shown())
+            self.assertIs(self.get_output(signal), data)
+            self.assertFalse(getattr(self.widget.Error, lsignal).is_shown())
 
     def test_owns_errors(self):
         self.assertIsNot(self.widget.Error, OWWidget.Error)
@@ -89,27 +90,27 @@ def test_multiple_signals(self):
         self.assertIsNone(console_locals["in_data"])
         self.assertEqual(console_locals["in_datas"], [])
 
-        self.send_signal("in_data", self.iris, (1, ))
+        self.send_signal("Data", self.iris, (1, ))
         click()
         self.assertIs(console_locals["in_data"], self.iris)
-        in_datas = console_locals["in_datas"]
-        self.assertEqual(len(in_datas), 1)
-        self.assertIs(in_datas[0], self.iris)
+        datas = console_locals["in_datas"]
+        self.assertEqual(len(datas), 1)
+        self.assertIs(datas[0], self.iris)
 
-        self.send_signal("in_data", titanic, (2, ))
+        self.send_signal("Data", titanic, (2, ))
         click()
         self.assertIsNone(console_locals["in_data"])
         self.assertEqual({id(obj) for obj in console_locals["in_datas"]},
                          {id(self.iris), id(titanic)})
 
-        self.send_signal("in_data", None, (2, ))
+        self.send_signal("Data", None, (2, ))
         click()
         self.assertIs(console_locals["in_data"], self.iris)
-        in_datas = console_locals["in_datas"]
-        self.assertEqual(len(in_datas), 1)
-        self.assertIs(in_datas[0], self.iris)
+        datas = console_locals["in_datas"]
+        self.assertEqual(len(datas), 1)
+        self.assertIs(datas[0], self.iris)
 
-        self.send_signal("in_data", None, (1, ))
+        self.send_signal("Data", None, (1, ))
         click()
         self.assertIsNone(console_locals["in_data"])
         self.assertEqual(console_locals["in_datas"], [])

Orange/widgets/tests/base.py

@@ -2,7 +2,6 @@
 import os
 import time
 import unittest
-import inspect
 from unittest.mock import Mock
 
 import numpy as np
@@ -43,6 +42,8 @@ def __init__(self):
         self.outputs = {}
 
     def send(self, widget, signal_name, value, id):
+        if not isinstance(signal_name, str):
+            signal_name = signal_name.name
         self.outputs[(widget, signal_name)] = value
 
 
@@ -217,10 +218,7 @@ def send_signal(self, input, value, *args, widget=None, wait=-1):
                 raise ValueError("'{}' is not an input name for widget {}"
                                  .format(input, type(widget).__name__))
         handler = getattr(widget, input.handler)
-        if "signal" in inspect.signature(handler).parameters:
-            handler(value, *args, signal=input.name)
-        else:
-            handler(value, *args)
+        handler(value, *args)
         widget.handleNewSignals()
         if wait >= 0 and widget.isBlocking():
             spy = QSignalSpy(widget.blockingStateChanged)