Add legend support for boxplots #27840

saranti · 2024-03-01T06:26:23Z

PR summary

Closes #27792. Boxplots now get legend entries.
This was previously attempted on #27711 and then reverted #27780 because it used the same parameter for tick labels and didn't generalize well. These 2 issues have been addressed in this PR.

A new label parameter has been added to Axes.boxplot and Axes.bxp. If called via ax.boxplot(), label is passed to the bxp function, otherwise bxp can be directly called with the label parameter.

The label is passed to the boxes object if showbox=True, otherwise it will be passed to the medians object is showbox=False. Attaching the label to the boxes makes sure that the legend handles will get a patch which will be displayed on the legend.
If the box is turned off, the legend handles will get a Line2D object from the median line which will be displayed as a line in the legend. This has 2 advantages:

It suits the aesthetics of a boxless plot
Unlike the box, caps, flyers and means, the medians cannot be turned off so they're a good vehicle for the legend label and will ensure that the labels always get set.

Click to old test cases

PR checklist

"closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
Plotting related features are demonstrated in an example
New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

saranti · 2024-03-02T07:45:52Z

Other things that need to be done at some stage:

Add an example. 3 possible options are:
- Adding a legend to https://matplotlib.org/devdocs/gallery/statistics/boxplot_color.html
- Replacing the basic legend in the second example in https://matplotlib.org/devdocs/gallery/statistics/boxplot_demo.html
- Add a whole new example plot

rcomer

Thanks for your work on this @saranti. It looks like you've done some good work pulling a variety of strands together.

One thing I'm a bit confused about is how the legend label is passed from boxplot to bxp. It is passed both as part of the boxprops dictionary and by itself. Why not just pass with the label parameter?

rcomer · 2024-03-03T18:23:19Z

doc/api/next_api_changes/deprecations/27840-TS.rst

@@ -0,0 +1,5 @@
+``boxplot`` legend labels
+~~~~~~~~~~~~~~~~~~~~~~~~~
+The tick labels on `~.Axes.boxplot` were previously set with the `labels` parameter.


Suggested change

The tick labels on `~.Axes.boxplot` were previously set with the `labels` parameter.

The tick labels on `~.Axes.boxplot` were previously set with the *labels* parameter.

When you use backticks, Sphinx tries to turn the word into a link. We have a convention to use emphasis for parameters.

You're right, the boxprops label is redundant. When i first implemented this, I used it as the only check to see if labels has been set but then I realized that it should've been set at the bxp function. Everything in the 'if boxprops' condition is no longer needed.

rcomer · 2024-03-03T18:25:09Z

doc/users/next_whats_new/boxplot_legend_support.rst

+Legend support for Boxplot
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+Boxplots now generate legend entries and can be labelled with the
+new `label` parameter. If a patch is passed to the box with `show_patch=True`,


Suggested change

new `label` parameter. If a patch is passed to the box with `show_patch=True`,

new *label* parameter. If a patch is passed to the box with ``show_patch=True``,

Double backtick gets you code formatting.

rcomer · 2024-03-03T18:29:13Z

lib/matplotlib/axes/_axes.py

-        labels : sequence, optional
+        tick_labels : sequence, optional
            Labels for each dataset (one per dataset). These are used for
            x-tick labels; *not* for legend entries.


Since the parameter now has a more specific name, I think we no longer need the note about them not being for the legend.

rcomer · 2024-03-03T18:32:03Z

lib/matplotlib/axes/_axes.py

+        label : sequence, optional
+            Legend labels for each boxplot.
+
+            .. versionadded:: 3.9


IIRC the data parameter is special and should always come last.

rcomer · 2024-03-03T18:50:06Z

lib/matplotlib/axes/_axes.py

+                box_or_whis = boxes if showbox else whiskers
+                for index, element in enumerate(box_or_whis):
+                    try:
+                        if isinstance(boxprops['label'], list):


I think is_scalar_or_string may be useful here.

rcomer · 2024-03-03T18:59:07Z

lib/matplotlib/tests/test_legend.py

+    with pytest.warns(mpl.MatplotlibDeprecationWarning,
+                      match='has been renamed \'tick_labels\''):
+        first = axs[0, 0].boxplot(data,
+                                  labels=labels,  # `tick_labels`


Maybe use tick_labels here rather than the deprecated name?

If I don't use the deprecated name then this pytest will fail.

rcomer · 2024-03-03T19:17:19Z

lib/matplotlib/tests/test_legend.py

+
+    # This plot will not generate a legend.
+    with pytest.warns(UserWarning, match='No artists with labels found'):
+        warnings.warn('No artists with labels found.', UserWarning)


I think this might be a stray debugging line?

Yes it is. I'm having trouble trying to get pytest to catch the UserWarning when legend is called without the labels being set. The list of emitted warnings is always null even though the warning is clearly raised in

matplotlib/lib/matplotlib/legend.py

Lines 1364 to 1370 in 013757e

elif len(args) == 0: # 0 args: automatically detect labels and handles.

handles, labels = _get_legend_handles_labels(axs, handlers)

if not handles:

_api.warn_external(

"No artists with labels found to put in legend. Note that "

"artists whose label start with an underscore are ignored "

"when legend() is called with no argument.")

rcomer · 2024-03-03T19:19:20Z

lib/matplotlib/tests/test_legend.py

+                                   showmeans=True,
+                                   patch_artist=True)
+        axs[0, 1].set_title('Tick labels, no legend')
+        axs[0, 1].legend()


It might be good for this test to get hold of the legend handles and labels and assert that these lists are empty.

lib/matplotlib/tests/test_legend.py

rcomer · 2024-03-03T19:40:14Z

lib/matplotlib/axes/_axes.py

            The style of the mean.
        data : indexable object, optional
            DATA_PARAMETER_PLACEHOLDER
+        label : sequence, optional


I think this could be a single string or a sequence?

This might actually need to be str or list of str

rcomer · 2024-03-04T17:47:11Z

The tests failed because of a problem with a new version of pytest which has now been yanked. I’m going to close and re-open to restart the CI.

timhoffm · 2024-03-04T19:19:34Z

AFAICT, this needs a rebase to pick up the pytest pinning.

saranti · 2024-03-06T03:53:22Z

@melissawm Thanks for the help yesterday, the rebase seems to have gone fine. The only problem I see now is that all the code having to do with the violin plot shouldn't be there and belongs to #27815

rcomer · 2024-03-08T08:30:49Z

@saranti your branch is still 60 commits behind main. I think you just need

git fetch upstream
git rebase upstream/main

to get it up to date. That should also get rid of the violinplot changes.

timhoffm

I feel uncomfortable with the scope of the PR - which has prevented me from looking more closely. It mixes the two independent topics legend support and parameter rename. I would I encourage separate PRs or at least separate commits to make it clear which feature is responsible for which code changes, and to ease review.

doc/api/next_api_changes/deprecations/27840-TS.rst

saranti · 2024-03-15T05:59:55Z

The old tick label test was still there after doing git pull upstream. I removed it as part of the conflict resolution but it's showing up in the diff 🤔

rcomer · 2024-03-15T08:05:34Z

I’ve never used git pull upstream so not really sure what that does. To get up to date with changes that have been merged to main, it is recommended to use rebase.
https://matplotlib.org/devdocs/devel/development_workflow.html#rebase-onto-upstream-main

lib/matplotlib/tests/test_legend.py

timhoffm

To fix the doc build.

lib/matplotlib/axes/_axes.py

timhoffm · 2024-03-19T06:27:03Z

Milestoning as 3.9 because it would be nice to get this in and IMO it’s ready. Just needs a second review.

doc/users/next_whats_new/boxplot_legend_support.rst

lib/matplotlib/axes/_axes.py

QuLogic · 2024-03-19T08:28:51Z

lib/matplotlib/axes/_axes.py

+                    raise ValueError("There must be an equal number of legend"
+                                     " labels and boxplots.")


This check should probably be in the "input validation" section, and reuse datashape_message.

lib/matplotlib/axes/_axes.pyi

github-actions bot added topic: pyplot API status: needs rebase labels Mar 1, 2024

saranti force-pushed the box_legend_support branch from 3cfd4ab to 68edfcb Compare March 2, 2024 02:58

github-actions bot removed the status: needs rebase label Mar 2, 2024

saranti marked this pull request as draft March 3, 2024 04:30

saranti force-pushed the box_legend_support branch from b03b849 to 9c033d2 Compare March 3, 2024 11:55

github-actions bot added status: needs rebase and removed status: needs rebase labels Mar 3, 2024

saranti marked this pull request as ready for review March 3, 2024 12:09

rcomer reviewed Mar 3, 2024

View reviewed changes

rcomer added topic: ticks axis labels topic: legend labels Mar 3, 2024

github-actions bot removed topic: ticks axis labels topic: legend labels Mar 4, 2024

rcomer closed this Mar 4, 2024

rcomer reopened this Mar 4, 2024

saranti force-pushed the box_legend_support branch from 008c0a5 to dd84a56 Compare March 5, 2024 13:16

github-actions bot added the Documentation: examples files in galleries/examples label Mar 5, 2024

saranti marked this pull request as draft March 5, 2024 13:17

saranti force-pushed the box_legend_support branch from dd84a56 to 0b5d5d6 Compare March 6, 2024 03:18

saranti force-pushed the box_legend_support branch from 0b5d5d6 to e1bb638 Compare March 8, 2024 08:34

github-actions bot removed the Documentation: examples files in galleries/examples label Mar 8, 2024

saranti marked this pull request as ready for review March 8, 2024 08:47

timhoffm reviewed Mar 8, 2024

View reviewed changes

doc/api/next_api_changes/deprecations/27840-TS.rst Outdated Show resolved Hide resolved

doc/api/next_api_changes/deprecations/27840-TS.rst Outdated Show resolved Hide resolved

saranti marked this pull request as draft March 9, 2024 03:42

github-actions bot removed the status: needs rebase label Mar 15, 2024

saranti force-pushed the box_legend_support branch from 9fcf0c0 to 73f2933 Compare March 15, 2024 05:14

github-actions bot added the status: needs rebase label Mar 15, 2024

saranti force-pushed the box_legend_support branch from 73f2933 to cae8a5d Compare March 15, 2024 05:17

github-actions bot removed the status: needs rebase label Mar 15, 2024

saranti force-pushed the box_legend_support branch 2 times, most recently from 9fcf0c0 to c17906f Compare March 15, 2024 05:50

saranti force-pushed the box_legend_support branch 2 times, most recently from 1902592 to 9fcf0c0 Compare March 15, 2024 08:38

timhoffm reviewed Mar 15, 2024

View reviewed changes

lib/matplotlib/tests/test_legend.py Outdated Show resolved Hide resolved

saranti force-pushed the box_legend_support branch from 9fcf0c0 to 14ddd7e Compare March 16, 2024 04:29

github-actions bot added the status: needs rebase label Mar 16, 2024

add legend to boxplot

95890c2

saranti force-pushed the box_legend_support branch from 14ddd7e to 95890c2 Compare March 16, 2024 04:35

github-actions bot removed the status: needs rebase label Mar 16, 2024

delete old function

7b39755

saranti force-pushed the box_legend_support branch from 5170c2d to 7b39755 Compare March 16, 2024 04:40

timhoffm approved these changes Mar 16, 2024

View reviewed changes

timhoffm reviewed Mar 18, 2024

View reviewed changes

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

fix doc build

cb2d3d1

timhoffm added this to the v3.9.0 milestone Mar 19, 2024

QuLogic reviewed Mar 19, 2024

View reviewed changes

saranti added 2 commits March 19, 2024 23:28

update and fix docs

2e41cb5

fix failing test

c96334d

saranti force-pushed the box_legend_support branch from 6004931 to c96334d Compare March 19, 2024 13:49

QuLogic merged commit 102d0a6 into matplotlib:main Mar 21, 2024

saranti deleted the box_legend_support branch March 23, 2024 05:28

	The tick labels on `~.Axes.boxplot` were previously set with the `labels` parameter.
	The tick labels on `~.Axes.boxplot` were previously set with the labels parameter.

	new `label` parameter. If a patch is passed to the box with `show_patch=True`,
	new label parameter. If a patch is passed to the box with ``show_patch=True``,

	elif len(args) == 0: # 0 args: automatically detect labels and handles.
	handles, labels = _get_legend_handles_labels(axs, handlers)
	if not handles:
	_api.warn_external(
	"No artists with labels found to put in legend. Note that "
	"artists whose label start with an underscore are ignored "
	"when legend() is called with no argument.")

		raise ValueError("There must be an equal number of legend"
		" labels and boxplots.")

Uh oh!

Add legend support for boxplots #27840

Add legend support for boxplots #27840

Uh oh!

Conversation

saranti commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

PR checklist

Uh oh!

saranti commented Mar 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rcomer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rcomer commented Mar 4, 2024

Uh oh!

timhoffm commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saranti commented Mar 6, 2024

Uh oh!

rcomer commented Mar 8, 2024

Uh oh!

timhoffm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

saranti commented Mar 15, 2024

Uh oh!

rcomer commented Mar 15, 2024

Uh oh!

Uh oh!

timhoffm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

timhoffm commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

saranti commented Mar 1, 2024 •

edited

Loading

saranti commented Mar 2, 2024 •

edited

Loading

timhoffm commented Mar 4, 2024 •

edited

Loading

timhoffm commented Mar 19, 2024 •

edited

Loading