From 0903406e7141dba5b6ce423f3e077566661daf74 Mon Sep 17 00:00:00 2001 From: janezd Date: Tue, 1 Aug 2017 15:49:31 +0200 Subject: [PATCH] OWPythonScript: Explicit is better than implicit --- Orange/canvas/scheme/widgetsscheme.py | 5 +- Orange/widgets/data/owpythonscript.py | 92 ++++++++++--------- .../widgets/data/tests/test_owpythonscript.py | 81 ++++++++-------- Orange/widgets/tests/base.py | 8 +- 4 files changed, 93 insertions(+), 93 deletions(-) diff --git a/Orange/canvas/scheme/widgetsscheme.py b/Orange/canvas/scheme/widgetsscheme.py index b502b6c73ff..abd00ffa963 100644 --- a/Orange/canvas/scheme/widgetsscheme.py +++ b/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'", diff --git a/Orange/widgets/data/owpythonscript.py b/Orange/widgets/data/owpythonscript.py index 581ccf1f24d..fa3a022c5de 100644 --- a/Orange/widgets/data/owpythonscript.py +++ b/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, "

Execute python script.

Input variables:

Output variables:

" ) @@ -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, diff --git a/Orange/widgets/data/tests/test_owpythonscript.py b/Orange/widgets/data/tests/test_owpythonscript.py index 1c4547a723b..e4a42940edf 100644 --- a/Orange/widgets/data/tests/test_owpythonscript.py +++ b/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"], []) diff --git a/Orange/widgets/tests/base.py b/Orange/widgets/tests/base.py index 521d5b5ecaf..f7179e21949 100644 --- a/Orange/widgets/tests/base.py +++ b/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)