diff --git a/Orange/widgets/data/owfeatureconstructor.py b/Orange/widgets/data/owfeatureconstructor.py index d32777d58b..5c22b33876 100644 --- a/Orange/widgets/data/owfeatureconstructor.py +++ b/Orange/widgets/data/owfeatureconstructor.py @@ -40,7 +40,7 @@ from Orange.util import frompyfunc from Orange.data import Variable, Table, Value, Instance from Orange.widgets import gui -from Orange.widgets.settings import ContextSetting, DomainContextHandler +from Orange.widgets.settings import Setting, DomainContextHandler from Orange.widgets.utils import ( itemmodels, vartype, ftry, unique_everseen as unique ) @@ -476,35 +476,29 @@ def freevars(exp: ast.AST, env: List[str]): raise ValueError(exp) -class FeatureConstructorHandler(DomainContextHandler): - """Context handler that filters descriptors""" - - def is_valid_item(self, setting, item, attrs, metas): - """Check if descriptor `item` can be used with given domain. - - Return True if descriptor's expression contains only - available variables and descriptors name does not clash with - existing variables. - """ - if item.name in attrs or item.name in metas: - return False - - try: - exp_ast = ast.parse(item.expression, mode="eval") - # ast.parse can return arbitrary errors, not only SyntaxError - # pylint: disable=broad-except - except Exception: - return False - - available = dict(globals()["__GLOBALS"]) - for var in attrs: - available[sanitized_name(var)] = None - for var in metas: - available[sanitized_name(var)] = None - - if freevars(exp_ast, list(available)): - return False - return True +class _FeatureConstructorHandler(DomainContextHandler): + """ContextHandler for backwards compatibility only. + This widget used to have context dependent settings. This ensures the + last stored context is propagated to regular settings instead. + """ + MAX_SAVED_CONTEXTS = 1 + + def initialize(self, instance, data=None): + super().initialize(instance, data) + if instance.context_settings: + # Use the very last context + ctx = instance.context_settings[0] + def pick_first(item): + # Return first element of item if item is a tuple + if isinstance(item, tuple): + return item[0] + else: + return item + instance.descriptors = ctx.values.get("descriptors", []) + instance.expressions_with_values = pick_first( + ctx.values.get("expressions_with_values", False) + ) + instance.currentIndex = pick_first(ctx.values.get("currentIndex", -1)) class OWFeatureConstructor(OWWidget, ConcurrentWidgetMixin): @@ -524,11 +518,13 @@ class Outputs: want_main_area = False - settingsHandler = FeatureConstructorHandler() - descriptors = ContextSetting([]) - currentIndex = ContextSetting(-1) - expressions_with_values = ContextSetting(False) - settings_version = 3 + # NOTE: The context handler is here for settings migration only. + settingsHandler = _FeatureConstructorHandler() + descriptors = Setting([], schema_only=True) + expressions_with_values = Setting(False, schema_only=True) + currentIndex = Setting(-1, schema_only=True) + + settings_version = 4 EDITORS = [ (ContinuousDescriptor, ContinuousFeatureEditor), @@ -660,6 +656,9 @@ def generate_newname(fmt): self.fix_button.setHidden(True) gui.button(self.buttonsArea, self, "Send", callback=self.apply, default=True) + if self.descriptors: + self.featuremodel[:] = list(self.descriptors) + def setCurrentIndex(self, index): index = min(index, len(self.featuremodel) - 1) self.currentIndex = index @@ -705,24 +704,9 @@ def setDescriptors(self, descriptors): @check_sql_input def setData(self, data=None): """Set the input dataset.""" - self.closeContext() - self.data = data - self.expressions_with_values = False - - self.descriptors = [] - self.currentIndex = -1 - if self.data is not None: - self.openContext(data) - - # disconnect from the selection model while reseting the model - selmodel = self.featureview.selectionModel() - selmodel.selectionChanged.disconnect(self._on_selectedVariableChanged) - - self.featuremodel[:] = list(self.descriptors) - self.setCurrentIndex(self.currentIndex) + self.setCurrentIndex(self.currentIndex) # Update editor - selmodel.selectionChanged.connect(self._on_selectedVariableChanged) self.fix_button.setHidden(not self.expressions_with_values) self.editorstack.setEnabled(self.currentIndex >= 0) @@ -819,12 +803,11 @@ def on_done(self, result: "Result") -> None: self.Outputs.data.send(data) def on_exception(self, ex: Exception): - log = logging.getLogger(__name__) - log.error("", exc_info=ex) self.Error.transform_error( "".join(format_exception_only(type(ex), ex)).rstrip(), exc_info=ex ) + self.Outputs.data.send(None) def on_partial_result(self, _): pass diff --git a/Orange/widgets/data/tests/test_owfeatureconstructor.py b/Orange/widgets/data/tests/test_owfeatureconstructor.py index 97faee6a36..ecca91de29 100644 --- a/Orange/widgets/data/tests/test_owfeatureconstructor.py +++ b/Orange/widgets/data/tests/test_owfeatureconstructor.py @@ -15,13 +15,12 @@ from Orange.data import (Table, Domain, StringVariable, ContinuousVariable, DiscreteVariable, TimeVariable) from Orange.widgets.tests.base import WidgetTest -from Orange.widgets.utils import vartype from Orange.widgets.utils.itemmodels import PyListModel from Orange.widgets.utils.concurrent import TaskState from Orange.widgets.data.owfeatureconstructor import ( DiscreteDescriptor, ContinuousDescriptor, StringDescriptor, construct_variables, OWFeatureConstructor, - FeatureEditor, DiscreteFeatureEditor, FeatureConstructorHandler, + FeatureEditor, DiscreteFeatureEditor, DateTimeDescriptor, StringFeatureEditor, freevars, validate_exp, FeatureFunc, run ) @@ -548,27 +547,42 @@ def test_migration_discrete_strings(self): self.assertTrue(widget.expressions_with_values) self.assertFalse(widget.fix_button.isHidden()) self.send_signal(widget.Inputs.data, None) - self.assertFalse(widget.expressions_with_values) self.assertTrue(widget.fix_button.isHidden()) + self.send_signal(widget.Inputs.data, data) + self.assertFalse(widget.fix_button.isHidden()) - def test_report(self): + def test_migration_no_context(self): + descriptors = [ + ContinuousDescriptor("y", "A + B", 1), + StringDescriptor("u", "str(A) + 'X'") + ] settings = { "context_settings": [Context( - attributes=dict(x=2, y=2, z=2), metas={}, + attributes=dict(A=1, B=2), metas={}, values=dict( - descriptors=[ - ContinuousDescriptor("a", "x + 2", 1), - DiscreteDescriptor("b", "x < 3", (), False), - DiscreteDescriptor("c", "x > 15", (), True), - DiscreteDescriptor("d", "y > x", ("foo", "bar"), False), - DiscreteDescriptor("e", "x ** 2 + y == 5", ("foo", "bar"), True), - StringDescriptor("f", "str(x)"), - DateTimeDescriptor("g", "z") - ], - currentIndex=0) + descriptors=descriptors, + currentIndex=1) )] } + w = self.create_widget(OWFeatureConstructor, settings) + self.assertEqual(w.descriptors, descriptors) + self.assertEqual(w.currentIndex, 1) + self.assertEqual(w.expressions_with_values, True) + + def test_report(self): + settings = { + "descriptors": [ + ContinuousDescriptor("a", "x + 2", 1), + DiscreteDescriptor("b", "x < 3", (), False), + DiscreteDescriptor("c", "x > 15", (), True), + DiscreteDescriptor("d", "y > x", ("foo", "bar"), False), + DiscreteDescriptor("e", "x ** 2 + y == 5", ("foo", "bar"), True), + StringDescriptor("f", "str(x)"), + DateTimeDescriptor("g", "z") + ], + "currentIndex": 0 + } w = self.create_widget(OWFeatureConstructor, settings) v = [ContinuousVariable(name) for name in "xyz"] @@ -610,47 +624,6 @@ def test_has_functions(self): self.assertIs(FeatureEditor.FUNCTIONS["sqrt"], math.sqrt) -class FeatureConstructorHandlerTests(unittest.TestCase): - def test_handles_builtins_in_expression(self): - self.assertTrue( - FeatureConstructorHandler().is_valid_item( - OWFeatureConstructor.descriptors, - StringDescriptor("X", "str(A) + str(B)"), - {"A": vartype(DiscreteVariable)}, - {"B": vartype(DiscreteVariable)} - ) - ) - - # no variables is also ok - self.assertTrue( - FeatureConstructorHandler().is_valid_item( - OWFeatureConstructor.descriptors, - StringDescriptor("X", "str('foo')"), - {}, - {} - ) - ) - - # should fail on unknown variables - self.assertFalse( - FeatureConstructorHandler().is_valid_item( - OWFeatureConstructor.descriptors, - StringDescriptor("X", "str(X)"), - {}, - {} - ) - ) - - def test_handles_special_characters_in_var_names(self): - self.assertTrue( - FeatureConstructorHandler().is_valid_item( - OWFeatureConstructor.descriptors, - StringDescriptor("X", "A_2_f"), - {"A.2 f": vartype(DiscreteVariable)}, - {} - ) - ) - if __name__ == "__main__": unittest.main()