Skip to content

Commit c9e5a76

Browse files
janezdjanezd
committed
OWMergData: Increase test coverage
1 parent f4461a6 commit c9e5a76

File tree

2 files changed

+190
-27
lines changed

2 files changed

+190
-27
lines changed

‎Orange/widgets/data/owmergedata.py‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def __init__(self, parent, model_left, model_right, pre_label, in_label):
4040
self.layout().setSpacing(0)
4141
self.setMouseTracking(True)
4242

43-
def add_row(self, value_left=None, value_right=None):
43+
def add_row(self):
4444
def sync_combos():
4545
combo = self.sender()
4646
index = combo.currentIndex()
@@ -52,11 +52,9 @@ def sync_combos():
5252
other.setCurrentText(model[index])
5353
self.emit_list()
5454

55-
def get_combo(model, value):
55+
def get_combo(model):
5656
combo = QComboBox(self)
5757
combo.setModel(model)
58-
if value is not None:
59-
combo.setCurrentIndex(model.indexOf(value))
6058
# We use signal activated because it is triggered only on user
6159
# interaction, not programmatically.
6260
combo.activated.connect(sync_combos)
@@ -72,9 +70,9 @@ def get_button(label, callback):
7270
row = self.layout().count()
7371
row_items = self.RowItems(
7472
QLabel("and" if row else self.pre_label),
75-
get_combo(self.model_left, value_left),
73+
get_combo(self.model_left),
7674
QLabel(self.in_label),
77-
get_combo(self.model_right, value_right),
75+
get_combo(self.model_right),
7876
get_button("×", self.on_remove_row),
7977
get_button("+", self.on_add_row)
8078
)
@@ -109,19 +107,20 @@ def on_remove_row(self):
109107
self.emit_list()
110108

111109
def _reset_buttons(self):
110+
def endis(button, enable, text):
111+
button.setEnabled(enable)
112+
button.setText(text if enable else "")
113+
112114
self.rows[0].pre_label.setText(self.pre_label)
113-
self.rows[0].remove_button.setText("× "[len(self.rows) == 1])
114-
self.rows[-1].add_button.setText("+")
115-
if len(self.rows) > 1:
116-
self.rows[-2].add_button.setText(" ")
115+
single_row = len(self.rows) == 1
116+
endis(self.rows[0].remove_button, not single_row, "×")
117+
endis(self.rows[-1].add_button, True, "+")
118+
if not single_row:
119+
endis(self.rows[-2].add_button, False, "")
117120

118121
def current_state(self):
119122
def get_var(model, combo):
120-
index = combo.currentIndex()
121-
if 0 <= index < len(model):
122-
return model[index]
123-
else:
124-
return None
123+
return model[combo.currentIndex()]
125124

126125
return [(get_var(self.model_left, row.left_combo),
127126
get_var(self.model_right, row.right_combo))
@@ -282,8 +281,9 @@ def _restore_combo_current_items(self, side, prev_settings):
282281
[row.left_combo, row.right_combo][side], pair[side])
283282

284283
def handleNewSignals(self):
285-
if self.attr_pairs:
286-
self.boxes[self.merging].set_current_state(self.attr_pairs)
284+
if self.attr_pairs and self.data:
285+
# TODO: This doesn't work, but should after #3925 (remove make)
286+
self.attr_boxes.set_state(self.attr_pairs)
287287
# This is schema-only setting, so it should be single-shot
288288
# More complicated alternative: make it a context setting
289289
self.attr_pairs = []
@@ -364,10 +364,6 @@ def _check_pair_types(self, pairs):
364364
self.Error.matching_id_with_sth(
365365
self._get_col_name(({left, right} - {INSTANCEID}).pop()))
366366
return False
367-
if (isinstance(left, str) or isinstance(right, str)) \
368-
and left != right:
369-
self.Error.matching_position_with_sth_else()
370-
return False
371367
return True
372368

373369
@staticmethod
@@ -526,6 +522,7 @@ def migrate_settings(settings, version=None):
526522
del settings[f"attr_{oper}_data"]
527523
del settings[f"attr_{oper}_extra"]
528524

525+
529526
if __name__ == "__main__": # pragma: no cover
530527
WidgetPreview(OWMergeData).run(
531528
setData=Orange.data.Table("tests/data-gender-region"),

‎Orange/widgets/data/tests/test_owmergedata.py‎

Lines changed: 172 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Test methods with long descriptive names can omit docstrings
22
# pylint: disable=missing-docstring
33
from itertools import chain
4+
import unittest
45

56
import numpy as np
67
import scipy.sparse as sp
@@ -50,7 +51,7 @@ def test_input_remove(self):
5051
self.send_signal(self.widget.Inputs.data, None)
5152
self.send_signal(self.widget.Inputs.extra_data, None)
5253

53-
def test_combobox_items_inner(self):
54+
def test_combobox_items(self):
5455
"""Check if combo box content is properly set"""
5556
domainA, domainB = self.dataA.domain, self.dataB.domain
5657
row = self.widget.attr_boxes.rows[0]
@@ -77,6 +78,176 @@ def test_combobox_items_inner(self):
7778
self.assertListEqual(data_combo.model()[:], data_items)
7879
self.assertListEqual(extra_combo.model()[:], extra_items)
7980

81+
def test_combo_box_sync(self):
82+
row = self.widget.attr_boxes.rows[0]
83+
data_combo, extra_combo = row.left_combo, row.right_combo
84+
85+
self.send_signal(self.widget.Inputs.data, self.dataA)
86+
self.send_signal(self.widget.Inputs.extra_data, self.dataA)
87+
88+
extra_combo.setCurrentIndex(3)
89+
data_combo.setCurrentIndex(0)
90+
data_combo.activated.emit(0)
91+
self.assertEqual(data_combo.currentIndex(), 0)
92+
self.assertEqual(extra_combo.currentIndex(), 0)
93+
94+
data_combo.setCurrentIndex(1)
95+
data_combo.activated.emit(1)
96+
self.assertEqual(data_combo.currentIndex(), 1)
97+
self.assertEqual(extra_combo.currentIndex(), 1)
98+
99+
data_combo.setCurrentIndex(2)
100+
data_combo.activated.emit(2)
101+
self.assertEqual(data_combo.currentIndex(), 2)
102+
self.assertEqual(extra_combo.currentIndex(), 1)
103+
104+
extra_combo.setCurrentIndex(0)
105+
extra_combo.activated.emit(0)
106+
self.assertEqual(data_combo.currentIndex(), 0)
107+
self.assertEqual(extra_combo.currentIndex(), 0)
108+
109+
extra_combo.setCurrentIndex(1)
110+
extra_combo.activated.emit(1)
111+
self.assertEqual(data_combo.currentIndex(), 1)
112+
self.assertEqual(extra_combo.currentIndex(), 1)
113+
114+
def test_add_row_button(self):
115+
boxes = self.widget.attr_boxes
116+
boxes.set_state([(INSTANCEID, INSTANCEID), (INSTANCEID, INSTANCEID)])
117+
boxes.rows[-1].add_button.clicked.emit()
118+
self.assertEqual(len(boxes.rows), 3)
119+
self.assertEqual(boxes.layout().count(), 3)
120+
121+
def test_remove_row(self):
122+
widget = self.widget
123+
boxes = widget.attr_boxes
124+
var0, var1 = self.dataA.domain.attributes[:2]
125+
126+
self.send_signal(self.widget.Inputs.data, self.dataA)
127+
self.send_signal(self.widget.Inputs.extra_data, self.dataA)
128+
129+
boxes.set_state(
130+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID), (var0, var1)])
131+
for i, row in enumerate(boxes.rows):
132+
self.assertTrue(row.remove_button.isEnabled())
133+
self.assertEqual(row.remove_button.text(), "×")
134+
self.assertEqual(row.add_button.isEnabled(), i == 2)
135+
self.assertEqual(row.add_button.text(), ["", "+"][i == 2])
136+
137+
boxes.rows[1].remove_button.clicked.emit()
138+
self.assertEqual(boxes.current_state(), [(INDEX, INDEX), (var0, var1)])
139+
for i, row in enumerate(boxes.rows):
140+
self.assertTrue(row.remove_button.isEnabled())
141+
self.assertEqual(row.remove_button.text(), "×")
142+
self.assertEqual(row.add_button.isEnabled(), i == 1)
143+
self.assertEqual(row.add_button.text(), ["", "+"][i])
144+
145+
boxes.rows[1].remove_button.clicked.emit()
146+
self.assertEqual(boxes.current_state(), [(INDEX, INDEX)])
147+
row = boxes.rows[0]
148+
self.assertFalse(row.remove_button.isEnabled())
149+
self.assertEqual(row.remove_button.text(), "")
150+
self.assertTrue(row.add_button.isEnabled())
151+
self.assertEqual(row.add_button.text(), "+")
152+
153+
boxes.set_state(
154+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID), (var0, var1)])
155+
boxes.rows[2].remove_button.clicked.emit()
156+
self.assertEqual(
157+
boxes.current_state(), [(INDEX, INDEX), (INSTANCEID, INSTANCEID)])
158+
for i, row in enumerate(boxes.rows):
159+
self.assertTrue(row.remove_button.isEnabled())
160+
self.assertEqual(row.remove_button.text(), "×")
161+
self.assertEqual(row.add_button.isEnabled(), i == 1)
162+
self.assertEqual(row.add_button.text(), ["", "+"][i == 1])
163+
164+
def test_retrieve_settings(self):
165+
widget = self.widget
166+
boxes = widget.attr_boxes
167+
var0, var1 = self.dataA.domain.attributes[:2]
168+
169+
self.send_signal(self.widget.Inputs.data, self.dataA)
170+
self.send_signal(self.widget.Inputs.extra_data, self.dataA)
171+
172+
boxes.set_state(
173+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID), (var0, var1)])
174+
175+
settings = widget.settingsHandler.pack_data(widget)
176+
177+
widget2 = self.create_widget(OWMergeData, stored_settings=settings)
178+
widget2.attr_boxes.set_state([(INDEX, INDEX)])
179+
self.send_signals(
180+
[(widget2.Inputs.data, self.dataA),
181+
(widget2.Inputs.extra_data, self.dataA)],
182+
widget=widget2)
183+
self.assertEqual(
184+
widget2.attr_boxes.current_state(),
185+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID), (var0, var1)])
186+
187+
@unittest.skip("widget doesn't work this way, but could in the future")
188+
def test_switch_domain(self): # pragma: no cover
189+
widget = self.widget
190+
boxes = widget.attr_boxes
191+
var0, var1 = self.dataA.domain.attributes[:2]
192+
193+
self.send_signal(self.widget.Inputs.data, self.dataA)
194+
self.send_signal(self.widget.Inputs.extra_data, self.dataA)
195+
boxes.set_state(
196+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID), (var0, var1)])
197+
198+
self.send_signal(self.widget.Inputs.data, self.dataB)
199+
self.send_signal(self.widget.Inputs.extra_data, self.dataB)
200+
self.assertEqual(
201+
boxes.current_state(),
202+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID)])
203+
204+
self.send_signal(self.widget.Inputs.data, self.dataA)
205+
self.send_signal(self.widget.Inputs.extra_data, self.dataA)
206+
boxes.set_state([(var0, var1), (var1, var0)])
207+
208+
self.send_signal(self.widget.Inputs.data, self.dataB)
209+
self.send_signal(self.widget.Inputs.extra_data, self.dataB)
210+
self.assertEqual(boxes.current_state(), [(INDEX, INDEX)])
211+
212+
self.send_signal(self.widget.Inputs.data, self.dataA)
213+
self.send_signal(self.widget.Inputs.extra_data, self.dataA)
214+
boxes.set_state([(var1, var1), (var0, var0)])
215+
216+
domain2 = Domain(self.dataA.domain.attributes[:1],
217+
self.dataA.domain.class_var)
218+
data2 = self.dataA.transform(domain2)
219+
self.send_signal(self.widget.Inputs.data, data2)
220+
self.send_signal(self.widget.Inputs.extra_data, data2)
221+
self.assertEqual(boxes.current_state(), [(var0, var0)])
222+
223+
def test_domain_switch_cleanup(self):
224+
widget = self.widget
225+
boxes = widget.attr_boxes
226+
var0, var1 = self.dataA.domain.attributes[:2]
227+
228+
self.send_signal(self.widget.Inputs.data, self.dataA)
229+
self.send_signal(self.widget.Inputs.extra_data, self.dataA)
230+
boxes.set_state(
231+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID), (var0, var1)])
232+
233+
self.send_signal(self.widget.Inputs.data, self.dataB)
234+
self.send_signal(self.widget.Inputs.extra_data, self.dataB)
235+
self.assertEqual(
236+
boxes.current_state(),
237+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID)])
238+
239+
def test_report(self):
240+
widget = self.widget
241+
boxes = widget.attr_boxes
242+
var0, var1 = self.dataA.domain.attributes[:2]
243+
244+
self.send_signal(self.widget.Inputs.data, self.dataA)
245+
self.send_signal(self.widget.Inputs.extra_data, self.dataA)
246+
boxes.set_state(
247+
[(INDEX, INDEX), (INSTANCEID, INSTANCEID), (var0, var1)])
248+
widget.send_report()
249+
# Don't crash, that's it
250+
80251
def test_output_merge_by_ids_inner(self):
81252
"""Check output for merging option 'Find matching rows' by
82253
Source position (index)"""
@@ -374,11 +545,6 @@ def test_best_match(self):
374545
f"wrong attributes chosen for merge_type={i}")
375546

376547
def test_sparse(self):
377-
"""
378-
Merge should work with sparse.
379-
GH-2295
380-
GH-2155
381-
"""
382548
data = Table("iris")[::25]
383549
data_ed_dense = Table("titanic")[::300]
384550
data_ed_sparse = Table("titanic")[::300].to_sparse()

0 commit comments

Comments
 (0)