Skip to content

Commit

Permalink
[FIX] Formula: Remove context settings (biolab#6801)
Browse files Browse the repository at this point in the history
owfeatureconstructor: Remove domain context settings
  • Loading branch information
ales-erjavec authored Jun 26, 2024
1 parent 55905a4 commit 7e620b6
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 109 deletions.
89 changes: 36 additions & 53 deletions Orange/widgets/data/owfeatureconstructor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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):
Expand All @@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
85 changes: 29 additions & 56 deletions Orange/widgets/data/tests/test_owfeatureconstructor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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()

0 comments on commit 7e620b6

Please sign in to comment.