Skip to content

[FIX] Distributions: Fix selection output #6578

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

Merged
markotoplak merged 3 commits into from
Sep 20, 2023
Merged

[FIX] Distributions: Fix selection output #6578

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
6ce7f67
Distributions: Fix selection output and context
janezd Sep 18, 2023
884eae3
Distributions: Rename selection to selected_bars, add tests
janezd Sep 19, 2023
52a12f7
Distributions: Pylint
janezd Sep 19, 2023
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
145 changes: 104 additions & 41 deletions Orange/widgets/visualize/owdistributions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ def __init__(self, x, width, padding, freqs, colors, stacked, expanded,
self.padding = padding
self.stacked = stacked
self.expanded = expanded
self.__picture = None
self.polygon = None
self.hovered = False
self._tooltip = tooltip
Expand Down Expand Up @@ -271,10 +270,12 @@ class Warning(OWWidget.Warning):
ignored_nans = Msg("Data instances with missing values are ignored")

settingsHandler = settings.DomainContextHandler()
settings_version = 2

var = settings.ContextSetting(None)
cvar = settings.ContextSetting(None)
selection = settings.ContextSetting(set(), schema_only=True)
# number_of_bins must be a context setting because selection depends on it
selected_bars = settings.ContextSetting(set(), schema_only=True)
# number_of_bins must be a context setting because selected_bars depends on it
number_of_bins = settings.ContextSetting(5, schema_only=True)

fitted_distribution = settings.Setting(0)
Expand Down Expand Up @@ -311,6 +312,7 @@ def __init__(self):
self.curve_items = []
self.curve_descriptions = None
self.binnings = []
self.ordered_values = []

self.last_click_idx = None
self.drag_operation = self.DragNone
Expand Down Expand Up @@ -495,6 +497,7 @@ def _on_hide_bars_changed(self):
self.plot.update()

def _set_smoothing_visibility(self):
# false positive, pylint: disable=invalid-sequence-index
self.smoothing_box.setDisabled(
self.Fitters[self.fitted_distribution][1] is not AshCurve)

Expand Down Expand Up @@ -563,6 +566,7 @@ def replot(self):
self._set_axis_names()
self._update_controls_state()
self._call_plotting()
self._reduce_selection()
self._display_legend()
self.show_selection()

Expand All @@ -571,6 +575,8 @@ def _clear_plot(self):
self.plot_pdf.clear()
self.plot_mark.clear()
self.bar_items = []
self.ordered_values = []
self.last_click_idx = None
self.curve_items = []
self._legend.clear()
self._legend.hide()
Expand Down Expand Up @@ -628,11 +634,11 @@ def _disc_plot(self):
else:
order = np.arange(len(dist))

ordered_values = np.array(var.values)[order]
self.ploti.getAxis("bottom").setTicks([list(enumerate(ordered_values))])
self.ordered_values = list(np.array(var.values)[order])
self.ploti.getAxis("bottom").setTicks([list(enumerate(self.ordered_values))])

colors = [QColor(0, 128, 255)]
for i, freq, desc in zip(count(), dist[order], ordered_values):
for i, freq, desc in zip(count(), dist[order], self.ordered_values):
tooltip = \
"<p style='white-space:pre;'>" \
f"<b>{escape(desc)}</b>: {int(freq)} " \
Expand All @@ -650,13 +656,13 @@ def _disc_split_plot(self):
else:
order = np.arange(len(conts))

ordered_values = np.array(var.values)[order]
self.ploti.getAxis("bottom").setTicks([list(enumerate(ordered_values))])
self.ordered_values = list(np.array(var.values)[order])
self.ploti.getAxis("bottom").setTicks([list(enumerate(self.ordered_values))])

gcolors = [QColor(*col) for col in self.cvar.colors]
gvalues = self.cvar.values
total = len(self.data)
for i, freqs, desc in zip(count(), conts[order], ordered_values):
for i, freqs, desc in zip(count(), conts[order], self.ordered_values):
self._add_bar(
i - 0.5, 1, 0.1, freqs, gcolors,
stacked=self.stacked_columns, expanded=self.show_probs,
Expand All @@ -682,6 +688,7 @@ def _cont_plot(self):
for i, (x0, x1), freq in zip(count(), zip(x, x[1:]), y):
tot_freq += freq
desc = self.str_int(x0, x1, not i, i == lasti, unique)
self.ordered_values.append(desc)
tooltip = \
"<p style='white-space:pre;'>" \
f"<b>{escape(desc)}</b>: " \
Expand Down Expand Up @@ -731,6 +738,7 @@ def _cont_split_plot(self):
tot_freqs += freqs
plotfreqs = tot_freqs.copy() if self.cumulative_distr else freqs
desc = self.str_int(x0, x1, not i, i == lasti, unique)
self.ordered_values.append(desc)
bar_width = width if unique else x1 - x0
self._add_bar(
x0 + xoff, bar_width, 0 if self.stacked_columns else 0.1,
Expand Down Expand Up @@ -783,6 +791,7 @@ def str_params():

if not y.size:
return None, None
# false positive, pylint: disable=invalid-sequence-index
_, dist, names, str_names = self.Fitters[self.fitted_distribution]
fitted = dist.fit(y)
params = dict(zip(names, fitted))
Expand All @@ -795,6 +804,7 @@ def _plot_approximations(self, x0, x1, fitters, colors, prior_probs):
for y, (fitter, _) in zip(ys, fitters):
if fitter is None:
continue
# false positive, pylint: disable=invalid-sequence-index
if self.Fitters[self.fitted_distribution][1] is AshCurve:
y[:] = fitter(x, sigma=(22 - self.kde_smoothing) / 40)
else:
Expand Down Expand Up @@ -933,28 +943,29 @@ def str_int(self, x0, x1, first, last, unique=False):
# Selection

def _on_item_clicked(self, item, modifiers, drag):
def add_or_remove(idx, add):
def add_or_remove(value, add):
self.drag_operation = [self.DragRemove, self.DragAdd][add]
if add:
self.selection.add(idx)
self.selected_bars.add(value)
else:
if idx in self.selection:
if value in self.selected_bars:
# This can be False when removing with dragging and the
# mouse crosses unselected items
self.selection.remove(idx)
self.selected_bars.remove(value)

def add_range(add):
if self.last_click_idx is None:
add = True
idx_range = {idx}
idx_range = {self.ordered_values[idx]}
else:
from_idx, to_idx = sorted((self.last_click_idx, idx))
idx_range = set(range(from_idx, to_idx + 1))
idx_range = {self.ordered_values[idx]
for idx in range(from_idx, to_idx + 1)}
self.drag_operation = [self.DragRemove, self.DragAdd][add]
if add:
self.selection |= idx_range
self.selected_bars |= idx_range
else:
self.selection -= idx_range
self.selected_bars -= idx_range

self.key_operation = None
if item is None:
Expand All @@ -966,19 +977,20 @@ def add_range(add):
# Dragging has to add a range, otherwise fast dragging skips bars
add_range(self.drag_operation == self.DragAdd)
else:
value = self.ordered_values[idx]
if modifiers & Qt.ShiftModifier:
add_range(self.drag_operation == self.DragAdd)
elif modifiers & Qt.ControlModifier:
add_or_remove(idx, add=idx not in self.selection)
add_or_remove(value, add=value not in self.selected_bars)
else:
if self.selection == {idx}:
# Clicking on a single selected bar deselects it,
if self.selected_bars == {value}:
# Clicking on a single selected bar deselects it,
# but dragging from here will select
add_or_remove(idx, add=False)
add_or_remove(value, add=False)
self.drag_operation = self.DragAdd
else:
self.selection.clear()
add_or_remove(idx, add=True)
self.selected_bars.clear()
add_or_remove(value, add=True)
self.last_click_idx = idx

self.show_selection()
Expand All @@ -987,7 +999,7 @@ def _on_blank_clicked(self):
self.reset_select()

def reset_select(self):
self.selection.clear()
self.selected_bars.clear()
self.last_click_idx = None
self.drag_operation = None
self.key_operation = None
Expand Down Expand Up @@ -1047,61 +1059,74 @@ def _padding(i):

def grouped_selection(self):
return [[g[1] for g in group]
for _, group in groupby(enumerate(sorted(self.selection)),
for _, group in groupby(enumerate(sorted(map(self.ordered_values.index,
self.selected_bars))),
key=lambda x: x[1] - x[0])]
# Alternative:
# groups = []
# last = None
# for idx, value in enumerate(self.ordered_values):
# if value in self.selected_bars:
# if last is None:
# groups.append(last := [])
# last.append(idx)
# else:
# last = None
# return groups

def keyPressEvent(self, e):
def on_nothing_selected():
if e.key() == Qt.Key_Left:
self.last_click_idx = len(self.bar_items) - 1
else:
self.last_click_idx = 0
self.selection.add(self.last_click_idx)
self.selected_bars.add(self.ordered_values[self.last_click_idx])

def on_key_left():
if e.modifiers() & Qt.ShiftModifier:
if self.key_operation == Qt.Key_Right and first != last:
self.selection.remove(last)
self.selected_bars.remove(self.ordered_values[last])
self.last_click_idx = last - 1
elif first:
self.key_operation = Qt.Key_Left
self.selection.add(first - 1)
self.selected_bars.add(self.ordered_values[first - 1])
self.last_click_idx = first - 1
else:
self.selection.clear()
self.selected_bars.clear()
self.last_click_idx = max(first - 1, 0)
self.selection.add(self.last_click_idx)
self.selected_bars.add(self.ordered_values[self.last_click_idx])

def on_key_right():
if e.modifiers() & Qt.ShiftModifier:
if self.key_operation == Qt.Key_Left and first != last:
self.selection.remove(first)
self.selected_bars.remove(self.ordered_values[first])
self.last_click_idx = first + 1
elif not self._is_last_bar(last):
self.key_operation = Qt.Key_Right
self.selection.add(last + 1)
self.selected_bars.add(self.ordered_values[last + 1])
self.last_click_idx = last + 1
else:
self.selection.clear()
self.selected_bars.clear()
self.last_click_idx = min(last + 1, len(self.bar_items) - 1)
self.selection.add(self.last_click_idx)
self.selected_bars.add(self.ordered_values[self.last_click_idx])

if not self.is_valid or not self.bar_items \
or e.key() not in (Qt.Key_Left, Qt.Key_Right):
super().keyPressEvent(e)
return

prev_selection = self.selection.copy()
if not self.selection:
prev_selection = self.selected_bars.copy()
if not self.selected_bars:
on_nothing_selected()
else:
first, last = min(self.selection), max(self.selection)
sel_indices = list(map(self.ordered_values.index, self.selected_bars))
first, last = min(sel_indices), max(sel_indices)
if e.key() == Qt.Key_Left:
on_key_left()
else:
on_key_right()

if self.selection != prev_selection:
if self.selected_bars != prev_selection:
self.drag_operation = self.DragAdd
self.show_selection()
self.apply.deferred()
Expand All @@ -1111,6 +1136,36 @@ def keyReleaseEvent(self, ev):
self.key_operation = None
super().keyReleaseEvent(ev)

def _reduce_selection(self):
"""
Unselect any bars that no longer appear in the plot; migrate from ints

This function is called after plotting to remove any bars that have
been selected but are no longer plotted. This occurs in particular
when the widget receives new data with discrete variables that lack
some values.

This function also migrates from previous settings, which stored ints
instead of values. This migration requires bar labels and cannot be
(easily) done before plotting.
"""
if self.selected_bars \
and isinstance(next(iter(self.selected_bars)), int):
self.selected_bars = {
self.ordered_values[idx] for idx in self.selected_bars
if idx < len(self.ordered_values)}
else:
self.selected_bars = {value for value in self.selected_bars
if value in self.ordered_values}

@classmethod
def migrate_context(cls, context, version):
# settings_version 2 has `selected_bars: set[str]` instead of
# `selection: set[int]`. Actual migration can only be done after
# plotting (see `_reduce_selection`), but we need to rename the setting
# so that handler assigns it to the widget instance
if "selection" in context.values:
context.values["selected_bars"] = context.values["selection"]

# -----------------------------
# Output
Expand Down Expand Up @@ -1143,9 +1198,16 @@ def apply(self):
def _get_output_indices_disc(self):
group_indices = np.zeros(len(self.data), dtype=np.int32)
col = self.data.get_column(self.var)
for group_idx, val_idx in enumerate(self.selection, start=1):
group_indices[col == val_idx] = group_idx
values = [self.var.values[i] for i in self.selection]
group_idx = 1
values = []
# self.selected_bars is a set, so its order is random;
# we iterate through ordered_value to get the same order as in chart
for value in self.ordered_values:
if value not in self.selected_bars:
continue
group_indices[col == self.var.to_val(value)] = group_idx
group_idx += 1
values.append(value)
return group_indices, values

def _get_output_indices_cont(self):
Expand All @@ -1166,6 +1228,7 @@ def _get_output_indices_cont(self):
return group_indices, values

def _get_histogram_table(self):
# bar is OK; pylint: disable=disallowed-name
var_bin = DiscreteVariable("Bin", [bar.desc for bar in self.bar_items])
var_freq = ContinuousVariable("Count")
X = []
Expand Down
Loading