Skip to content

Use scatter for check boxes and set facecolors correctly in check boxes and radio buttons #24474

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
merged 7 commits into from
Dec 22, 2022
Merged

Use scatter for check boxes and set facecolors correctly in check boxes and radio buttons #24474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
723cd86
Use scatter for check boxes instead of Rectangle
chahak13 Nov 16, 2022
805f365
Specify size for the buttons to be same as radio
chahak13 Nov 16, 2022
61db294
Use correct setter to set facecolor in checkbox
chahak13 Nov 17, 2022
04d8fda
Update radio buttons to also use correct setter
chahak13 Nov 17, 2022
0f09fa1
Set rectangles and lines property for backcompat
chahak13 Nov 17, 2022
143cb9a
Create rectangles/lines when the other is accessed
chahak13 Nov 30, 2022
788a0d1
Add tests to check .rectangles and .lines fallback
chahak13 Dec 1, 2022
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
4 changes: 4 additions & 0 deletions doc/api/next_api_changes/deprecations/24474_CM.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
``CheckButtons.rectangles`` and ``CheckButtons.lines``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``CheckButtons.rectangles`` and ``CheckButtons.lines`` are deprecated.
(``CheckButtons`` now draws itself using `~.Axes.scatter`.)
4 changes: 2 additions & 2 deletions doc/api/prev_api_changes/api_changes_2.1.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,8 +422,8 @@ The ``shading`` kwarg to `~matplotlib.axes.Axes.pcolor` has been
removed. Set ``edgecolors`` appropriately instead.


Functions removed from the `.lines` module
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Functions removed from the ``lines`` module
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The :mod:`matplotlib.lines` module no longer imports the
``pts_to_prestep``, ``pts_to_midstep`` and ``pts_to_poststep``
Expand Down
69 changes: 67 additions & 2 deletions lib/matplotlib/tests/test_widgets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import matplotlib.colors as mcolors
import matplotlib.widgets as widgets
import matplotlib.pyplot as plt
from matplotlib.patches import Rectangle
from matplotlib.lines import Line2D
from matplotlib.testing.decorators import check_figures_equal, image_comparison
from matplotlib.testing.widgets import (click_and_drag, do_event, get_ax,
mock_event, noop)
Expand Down Expand Up @@ -1006,8 +1008,10 @@ def test_check_radio_buttons_image():
rb = widgets.RadioButtons(rax1, ('Radio 1', 'Radio 2', 'Radio 3'))
with pytest.warns(DeprecationWarning):
rb.circles # Trigger the old-style elliptic radiobuttons.
widgets.CheckButtons(rax2, ('Check 1', 'Check 2', 'Check 3'),
(False, True, True))
cb = widgets.CheckButtons(rax2, ('Check 1', 'Check 2', 'Check 3'),
(False, True, True))
with pytest.warns(DeprecationWarning):
cb.rectangles # Trigger old-style Rectangle check boxes


@check_figures_equal(extensions=["png"])
Expand All @@ -1020,6 +1024,67 @@ def test_radio_buttons(fig_test, fig_ref):
ax.text(.25, 1/3, "coffee", transform=ax.transAxes, va="center")


@check_figures_equal(extensions=["png"])
def test_check_buttons(fig_test, fig_ref):
widgets.CheckButtons(fig_test.subplots(), ["tea", "coffee"], [True, True])
ax = fig_ref.add_subplot(xticks=[], yticks=[])
ax.scatter([.15, .15], [2/3, 1/3], marker='s', transform=ax.transAxes,
s=(plt.rcParams["font.size"] / 2) ** 2, c=["none", "none"])
ax.scatter([.15, .15], [2/3, 1/3], marker='x', transform=ax.transAxes,
s=(plt.rcParams["font.size"] / 2) ** 2, c=["k", "k"])
ax.text(.25, 2/3, "tea", transform=ax.transAxes, va="center")
ax.text(.25, 1/3, "coffee", transform=ax.transAxes, va="center")


@check_figures_equal(extensions=["png"])
def test_check_buttons_rectangles(fig_test, fig_ref):
# Test should be removed once .rectangles is removed
cb = widgets.CheckButtons(fig_test.subplots(), ["", ""],
[False, False])
with pytest.warns(DeprecationWarning):
cb.rectangles
ax = fig_ref.add_subplot(xticks=[], yticks=[])
ys = [2/3, 1/3]
dy = 1/3
w, h = dy / 2, dy / 2
rectangles = [
Rectangle(xy=(0.05, ys[i] - h / 2), width=w, height=h,
edgecolor="black",
facecolor="none",
transform=ax.transAxes
)
for i, y in enumerate(ys)
]
for rectangle in rectangles:
ax.add_patch(rectangle)


@check_figures_equal(extensions=["png"])
def test_check_buttons_lines(fig_test, fig_ref):
# Test should be removed once .lines is removed
cb = widgets.CheckButtons(fig_test.subplots(), ["", ""], [True, True])
with pytest.warns(DeprecationWarning):
cb.lines
for rectangle in cb._rectangles:
rectangle.set_visible(False)
ax = fig_ref.add_subplot(xticks=[], yticks=[])
ys = [2/3, 1/3]
dy = 1/3
w, h = dy / 2, dy / 2
lineparams = {'color': 'k', 'linewidth': 1.25,
'transform': ax.transAxes,
'solid_capstyle': 'butt'}
for i, y in enumerate(ys):
x, y = 0.05, y - h / 2
l1 = Line2D([x, x + w], [y + h, y], **lineparams)
l2 = Line2D([x, x + w], [y, y + h], **lineparams)

l1.set_visible(True)
l2.set_visible(True)
ax.add_line(l1)
ax.add_line(l2)


def test_slider_slidermin_slidermax_invalid():
fig, ax = plt.subplots()
# test min/max with floats
Expand Down
153 changes: 107 additions & 46 deletions lib/matplotlib/widgets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1002,43 +1002,23 @@ def __init__(self, ax, labels, actives=None):
if actives is None:
actives = [False] * len(labels)

if len(labels) > 1:
dy = 1. / (len(labels) + 1)
ys = np.linspace(1 - dy, dy, len(labels))
else:
dy = 0.25
ys = [0.5]

axcolor = ax.get_facecolor()

self.labels = []
self.lines = []
self.rectangles = []

lineparams = {'color': 'k', 'linewidth': 1.25,
'transform': ax.transAxes, 'solid_capstyle': 'butt'}
for y, label, active in zip(ys, labels, actives):
t = ax.text(0.25, y, label, transform=ax.transAxes,
horizontalalignment='left',
verticalalignment='center')

w, h = dy / 2, dy / 2
x, y = 0.05, y - h / 2

p = Rectangle(xy=(x, y), width=w, height=h, edgecolor='black',
facecolor=axcolor, transform=ax.transAxes)
ys = np.linspace(1, 0, len(labels)+2)[1:-1]
text_size = mpl.rcParams["font.size"] / 2

l1 = Line2D([x, x + w], [y + h, y], **lineparams)
l2 = Line2D([x, x + w], [y, y + h], **lineparams)
self.labels = [
ax.text(0.25, y, label, transform=ax.transAxes,
horizontalalignment="left", verticalalignment="center")
for y, label in zip(ys, labels)]

l1.set_visible(active)
l2.set_visible(active)
self.labels.append(t)
self.rectangles.append(p)
self.lines.append((l1, l2))
ax.add_patch(p)
ax.add_line(l1)
ax.add_line(l2)
self._squares = ax.scatter(
[0.15] * len(ys), ys, marker='s', s=text_size**2,
c="none", linewidth=1, transform=ax.transAxes, edgecolor="k"
)
self._crosses = ax.scatter(
[0.15] * len(ys), ys, marker='x', linewidth=1, s=text_size**2,
c=["k" if active else "none" for active in actives],
transform=ax.transAxes
)

self.connect_event('button_press_event', self._clicked)

Expand All @@ -1047,11 +1027,27 @@ def __init__(self, ax, labels, actives=None):
def _clicked(self, event):
if self.ignore(event) or event.button != 1 or event.inaxes != self.ax:
return
for i, (p, t) in enumerate(zip(self.rectangles, self.labels)):
if (t.get_window_extent().contains(event.x, event.y) or
p.get_window_extent().contains(event.x, event.y)):
self.set_active(i)
break
pclicked = self.ax.transAxes.inverted().transform((event.x, event.y))
distances = {}
if hasattr(self, "_rectangles"):
for i, (p, t) in enumerate(zip(self._rectangles, self.labels)):
x0, y0 = p.get_xy()
if (t.get_window_extent().contains(event.x, event.y)
or (x0 <= pclicked[0] <= x0 + p.get_width()
and y0 <= pclicked[1] <= y0 + p.get_height())):
distances[i] = np.linalg.norm(pclicked - p.get_center())
else:
_, square_inds = self._squares.contains(event)
coords = self._squares.get_offset_transform().transform(
self._squares.get_offsets()
)
for i, t in enumerate(self.labels):
if (i in square_inds["ind"]
or t.get_window_extent().contains(event.x, event.y)):
distances[i] = np.linalg.norm(pclicked - coords[i])
if len(distances) > 0:
closest = min(distances, key=distances.get)
self.set_active(closest)

def set_active(self, index):
"""
Expand All @@ -1072,9 +1068,20 @@ def set_active(self, index):
if index not in range(len(self.labels)):
raise ValueError(f'Invalid CheckButton index: {index}')

l1, l2 = self.lines[index]
l1.set_visible(not l1.get_visible())
l2.set_visible(not l2.get_visible())
cross_facecolors = self._crosses.get_facecolor()
cross_facecolors[index] = colors.to_rgba(
"black"
if colors.same_color(
cross_facecolors[index], colors.to_rgba("none")
)
else "none"
)
self._crosses.set_facecolor(cross_facecolors)

if hasattr(self, "_lines"):
l1, l2 = self._lines[index]
l1.set_visible(not l1.get_visible())
l2.set_visible(not l2.get_visible())

if self.drawon:
self.ax.figure.canvas.draw()
Comment on lines 1086 to 1087
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anntzer I have a preliminary implementation for square check boxes but I'm running into one small issue. When I try running this, everything works as intended when we change the figure size (the check boxes remain square), but the unit test fails. For some reason, calling set_active(0) in code doesn't change the status of the checkboxes. The issue is arising in these lines. The status changes correctly based on the code that I added but after executing self.ax.figure.canvas.draw(), the colors for all the scatter markers change back to what they were initially set to be, and not the changed ones. This is happening only in the unit tests and not while actually checking it using a GUI backend. Any ideas/suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I'm missing something but can't see it right now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do some print-debugging and grep for places that could touch facecolors, you'll see that Collection.update_scalarmappable gets called at some point which causes the facecolors to be reset, but you'll need to figure out by yourself why (because I don't actually know immediately).

Copy link
Contributor Author

@chahak13 chahak13 Nov 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That helps, thanks! I'll try figuring out why this is happening. The main reason why I was getting confused was that it happened only when set_active was explicitly called but worked as intended with the actual figure. Should be able to figure this out now though, hopefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anntzer this is actually true for the radio buttons too! Small code to reproduce (based on the checkbox example)

radio = RadioButtons(rax, labels, active=1, activecolor="k")
print(radio.value_selected)
print(radio._buttons.get_facecolor())
print("Changing active button from 1 to 0")
radio.set_active(0)
print(radio.value_selected)
print(radio._buttons.get_facecolor())

Output:

4 Hz
[[0. 0. 0. 0.]
 [0. 0. 0. 1.]
 [0. 0. 0. 0.]]
Changing active button from 1 to 0
2 Hz
[[0. 0. 0. 0.]
 [0. 0. 0. 1.]
 [0. 0. 0. 0.]]

This shows that the facecolors for the buttons don't change even after set_active(0) is called, even though value_selected is correct. The problem arises because of these lines

matplotlib/lib/matplotlib/collections.py

Lines 842 to 845 in a9c9620

# The flags are initialized to None to ensure this returns True
# the first time it is called.
edge0 = self._edge_is_mapped
face0 = self._face_is_mapped

Since it sets the flags such that it returns True the first time,

matplotlib/lib/matplotlib/collections.py

Lines 894 to 900 in a9c9620

else:
self._set_facecolor(self._original_facecolor)
if self._edge_is_mapped:
self._edgecolors = self._mapped_colors
else:
self._set_edgecolor(self._original_edgecolor)
self.stale = True

set the original colors again. Since update_scalarmappable gets called the first time they're drawn, when creating a figure, all functionalities work as everything works as expected after the second call. Even in this case, if there are any set_active calls after the first one, they work fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the investigation, I have opened #24479 to track this. Sorry for sending you down this rabbit hole, perhaps this was not so easy after all...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it was fun. But now with the question on hand, this bug stops me to run get_status for the check boxes correctly. I guess one workaround could be that I maintain an active array similar to value_active in radio buttons but that'll be a hack and I'm on the fence about having an extra array to handle that when it can be done with the colors itself. That'll pass the tests though, and everything works in a figure anyway. Suggestions?

Expand All @@ -1086,7 +1093,8 @@ def get_status(self):
"""
Return a tuple of the status (True/False) of all of the check buttons.
"""
return [l1.get_visible() for (l1, l2) in self.lines]
return [not colors.same_color(color, colors.to_rgba("none"))
for color in self._crosses.get_facecolors()]

def on_clicked(self, func):
"""
Expand All @@ -1100,6 +1108,57 @@ def disconnect(self, cid):
"""Remove the observer with connection id *cid*."""
self._observers.disconnect(cid)

@_api.deprecated("3.7")
@property
def rectangles(self):
if not hasattr(self, "_rectangles"):
ys = np.linspace(1, 0, len(self.labels)+2)[1:-1]
dy = 1. / (len(self.labels) + 1)
w, h = dy / 2, dy / 2
rectangles = self._rectangles = [
Rectangle(xy=(0.05, ys[i] - h / 2), width=w, height=h,
edgecolor="black",
facecolor="none",
transform=self.ax.transAxes
)
for i, y in enumerate(ys)
]
self._squares.set_visible(False)
for rectangle in rectangles:
self.ax.add_patch(rectangle)
if not hasattr(self, "_lines"):
with _api.suppress_matplotlib_deprecation_warning():
_ = self.lines
return self._rectangles

@_api.deprecated("3.7")
@property
def lines(self):
if not hasattr(self, "_lines"):
ys = np.linspace(1, 0, len(self.labels)+2)[1:-1]
self._crosses.set_visible(False)
dy = 1. / (len(self.labels) + 1)
w, h = dy / 2, dy / 2
self._lines = []
current_status = self.get_status()
lineparams = {'color': 'k', 'linewidth': 1.25,
'transform': self.ax.transAxes,
'solid_capstyle': 'butt'}
for i, y in enumerate(ys):
x, y = 0.05, y - h / 2
l1 = Line2D([x, x + w], [y + h, y], **lineparams)
l2 = Line2D([x, x + w], [y, y + h], **lineparams)

l1.set_visible(current_status[i])
l2.set_visible(current_status[i])
self._lines.append((l1, l2))
self.ax.add_patch(l1)
self.ax.add_patch(l2)
if not hasattr(self, "_rectangles"):
with _api.suppress_matplotlib_deprecation_warning():
_ = self.rectangles
return self._lines


class TextBox(AxesWidget):
"""
Expand Down Expand Up @@ -1457,8 +1516,10 @@ def set_active(self, index):
if index not in range(len(self.labels)):
raise ValueError(f'Invalid RadioButton index: {index}')
self.value_selected = self.labels[index].get_text()
self._buttons.get_facecolor()[:] = colors.to_rgba("none")
self._buttons.get_facecolor()[index] = colors.to_rgba(self.activecolor)
button_facecolors = self._buttons.get_facecolor()
button_facecolors[:] = colors.to_rgba("none")
button_facecolors[index] = colors.to_rgba(self.activecolor)
self._buttons.set_facecolor(button_facecolors)
if hasattr(self, "_circles"): # Remove once circles is removed.
for i, p in enumerate(self._circles):
p.set_facecolor(self.activecolor if i == index else "none")
Expand Down