Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FIX] Select Rows: Fix crash on changed variable type #4254

Merged
merged 3 commits into from
Dec 14, 2019
Merged

[FIX] Select Rows: Fix crash on changed variable type #4254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c69e6f
Select Rows: Fix crash on changed variable type
janezd Dec 6, 2019
7323aac
Select Rows: Add code for support of old settings and ensure its remo…
janezd Dec 6, 2019
fa537b1
Select Rows: Add test for partial matches
janezd Dec 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions Orange/widgets/data/owselectrows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,47 @@ def is_valid_item(self, setting, condition, attrs, metas):
return varname in attrs or varname in metas

def encode_setting(self, context, setting, value):
if setting.name == 'conditions':
CONTINUOUS = vartype(ContinuousVariable("x"))
for i, (attr, op, values) in enumerate(value):
if context.attributes.get(attr) == CONTINUOUS:
if values and isinstance(values[0], str):
values = [QLocale().toDouble(v)[0] for v in values]
value[i] = (attr, op, values)
return super().encode_setting(context, setting, value)
if setting.name != 'conditions':
return super().encode_settings(context, setting, value)

encoded = []
CONTINUOUS = vartype(ContinuousVariable("x"))
for attr, op, values in value:
vtype = context.attributes.get(attr)
if vtype == CONTINUOUS and values and isinstance(values[0], str):
values = [QLocale().toDouble(v)[0] for v in values]
encoded.append((attr, vtype, op, values))
return encoded

def decode_setting(self, setting, value, domain=None):
value = super().decode_setting(setting, value, domain)
if setting.name == 'conditions':
for i, (attr, op, values) in enumerate(value):
# Use this after 2022/2/2:
# for i, (attr, _, op, values) in enumerate(value):
for i, condition in enumerate(value):
attr = condition[0]
op, values = condition[-2:]

var = attr in domain and domain[attr]
if var and var.is_continuous and not isinstance(var, TimeVariable):
value[i] = (attr, op,
list([QLocale().toString(float(i), 'f')
for i in values]))
values = [QLocale().toString(float(i), 'f') for i in values]
value[i] = (attr, op, values)
return value

def match(self, context, domain, attrs, metas):
if (attrs, metas) == (context.attributes, context.metas):
return self.PERFECT_MATCH

conditions = context.values["conditions"]
all_vars = attrs
all_vars.update(metas)
# Use this after 2022/2/2:
# if all(all_vars.get(name) == tpe for name, tpe, *_ in conditions):
if all(all_vars.get(name) == tpe if len(rest) == 2 else name in all_vars
for name, tpe, *rest in conditions):
return 0.5
return self.NO_MATCH


class FilterDiscreteType(enum.Enum):
Equal = "Equal"
Expand Down Expand Up @@ -105,6 +126,8 @@ class Outputs:
purge_classes = Setting(False, schema_only=True)
auto_commit = Setting(True)

settings_version = 2

Operators = {
ContinuousVariable: [
(FilterContinuous.Equal, "equals"),
Expand Down Expand Up @@ -692,6 +715,14 @@ def send_report(self):
("Non-matching data",
nonmatch_inst > 0 and "{} instances".format(nonmatch_inst))))

# Uncomment this on 2022/2/2
#
# @classmethod
# def migrate_context(cls, context, version):
# if not version or version < 2:
# # Just remove; can't migrate because variables types are unknown
# context.values["conditions"] = []


class CheckBoxPopup(QWidget):
def __init__(self, var, lc, widget_parent=None, widget=None):
Expand Down
85 changes: 78 additions & 7 deletions Orange/widgets/data/tests/test_owselectrows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
# Test methods with long descriptive names can omit docstrings
# pylint: disable=missing-docstring
import time

from AnyQt.QtCore import QLocale, Qt
from AnyQt.QtTest import QTest
from AnyQt.QtWidgets import QLineEdit, QComboBox

import numpy as np

from Orange.data import (
Table, ContinuousVariable, StringVariable, DiscreteVariable)
Table, ContinuousVariable, StringVariable, DiscreteVariable, Domain)
from Orange.widgets.data.owselectrows import (
OWSelectRows, FilterDiscreteType, SelectRowsContextHandler)
from Orange.widgets.tests.base import WidgetTest, datasets
Expand All @@ -16,6 +18,7 @@
from Orange.widgets.tests.utils import simulate, override_locale
from Orange.widgets.utils.annotated_data import ANNOTATED_DATA_FEATURE_NAME
from Orange.tests import test_filename
from orangewidget.settings import VERSION_KEY

CFValues = {
FilterContinuous.Equal: ["5.4"],
Expand Down Expand Up @@ -132,22 +135,22 @@ def test_stores_settings_in_invariant_locale(self):
context = self.widget.current_context
self.send_signal(self.widget.Inputs.data, None)
saved_condition = context.values["conditions"][0]
self.assertEqual(saved_condition[2][0], 5.2)
self.assertEqual(saved_condition[3][0], 5.2)

@override_locale(QLocale.C)
def test_restores_continuous_filter_in_c_locale(self):
iris = Table("iris")[:5]
# Settings with string value
self.widget = self.widget_with_context(
iris.domain, [["sepal length", 2, ("5.2",)]])
iris.domain, [["sepal length", 102, 2, ("5.2",)]])
self.send_signal(self.widget.Inputs.data, iris)

values = self.widget.conditions[0][2]
self.assertTrue(values[0].startswith("5.2"))

# Settings with float value
self.widget = self.widget_with_context(
iris.domain, [["sepal length", 2, (5.2,)]])
iris.domain, [["sepal length", 102, 2, (5.2,)]])
self.send_signal(self.widget.Inputs.data, iris)

values = self.widget.conditions[0][2]
Expand All @@ -158,20 +161,33 @@ def test_restores_continuous_filter_in_sl_SI_locale(self):
iris = Table("iris")[:5]
# Settings with string value
self.widget = self.widget_with_context(
iris.domain, [["sepal length", 2, ("5.2",)]])
iris.domain, [["sepal length", 102, 2, ("5.2",)]])
self.send_signal(self.widget.Inputs.data, iris)

values = self.widget.conditions[0][2]
self.assertTrue(values[0].startswith("5,2"))

# Settings with float value
self.widget = self.widget_with_context(
iris.domain, [["sepal length", 2, (5.2,)]])
iris.domain, [["sepal length", 102, 2, (5.2,)]])
self.send_signal(self.widget.Inputs.data, iris)

values = self.widget.conditions[0][2]
self.assertTrue(values[0].startswith("5,2"))

@override_locale(QLocale.C)
def test_partial_matches(self):
iris = Table("iris")
domain = iris.domain
self.widget = self.widget_with_context(
domain, [[domain[0].name, 2, ("5.2",)]])
iris2 = iris.transform(Domain(domain.attributes[:2], None))
self.send_signal(self.widget.Inputs.data, iris2)
condition = self.widget.conditions[0]
self.assertEqual(condition[0], "sepal length")
self.assertEqual(condition[1], 2)
self.assertTrue(condition[2][0].startswith("5.2"))

def test_load_settings(self):
iris = Table("iris")[:5]
self.send_signal(self.widget.Inputs.data, iris)
Expand Down Expand Up @@ -242,10 +258,65 @@ def test_annotated_data(self):
np.testing.assert_equal(annotations[:50], True)
np.testing.assert_equal(annotations[50:], False)

def test_change_var_type(self):
iris = Table("iris")
domain = iris.domain

self.send_signal(self.widget.Inputs.data, iris)
self.widget.remove_all_button.click()
self.enterFilter(domain[0], "is below", "5.2")

var0vals = list({str(x) for x in iris.X[:, 0]})
new_domain = Domain(
(DiscreteVariable(domain[0].name, values=var0vals), )
+ domain.attributes[1:],
domain.class_var)
new_iris = iris.transform(new_domain)
self.send_signal(self.widget.Inputs.data, new_iris)

# Uncomment this on 2022/2/2
#
# def test_migration_to_version_1(self):
# iris = Table("iris")
#
# ch = SelectRowsContextHandler()
# context = ch.new_context(iris.domain, *ch.encode_domain(iris.domain))
# context.values = dict(conditions=[["petal length", 2, (5.2,)]])
# settings = dict(context_settings=[context])
# widget = self.create_widget(OWSelectRows, settings)
# self.assertEqual(widget.conditions, [])

@override_locale(QLocale.C)
def test_support_old_settings(self):
iris = Table("iris")
self.widget = self.widget_with_context(
iris.domain, [["sepal length", 2, ("5.2",)]])
self.send_signal(self.widget.Inputs.data, iris)
condition = self.widget.conditions[0]
self.assertEqual(condition[0], "sepal length")
self.assertEqual(condition[1], 2)
self.assertTrue(condition[2][0].startswith("5.2"))

def test_end_support_for_version_1(self):
if time.gmtime() >= (2022, 2, 2):
self.fail("""
Happy 22/2/2!

Now remove support for version==None settings in
SelectRowsContextHandler.decode_setting and SelectRowsContextHandler.match,
and uncomment OWSelectRows.migrate.

In tests, uncomment test_migration_to_version_1,
and remove test_support_old_settings and this test.

Basically, revert this commit.
""")

def widget_with_context(self, domain, conditions):
ch = SelectRowsContextHandler()
context = ch.new_context(domain, *ch.encode_domain(domain))
context.values = dict(conditions=conditions)
context.values = {"conditions": conditions,
VERSION_KEY: OWSelectRows.settings_version}
settings = dict(context_settings=[context])

return self.create_widget(OWSelectRows, settings)
Expand Down