From 1689526f0a781f35a2e675baa32a6e2aa2e264ff Mon Sep 17 00:00:00 2001 From: Michael Waskom Date: Tue, 5 Jul 2022 19:44:57 -0400 Subject: [PATCH 1/3] Improve visual appearance of Bar mark --- seaborn/_marks/bars.py | 47 ++++++++++++++++++++++++++++++++++----- tests/_marks/test_bars.py | 6 +++-- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/seaborn/_marks/bars.py b/seaborn/_marks/bars.py index 514686990f..208ff1cd3a 100644 --- a/seaborn/_marks/bars.py +++ b/seaborn/_marks/bars.py @@ -1,6 +1,7 @@ from __future__ import annotations from dataclasses import dataclass +import numpy as np import matplotlib as mpl from seaborn._marks.base import ( @@ -67,31 +68,67 @@ def coords_to_geometry(x, y, w, b): xy = b, y - h / 2 return xy, w, h + val_idx = ["y", "x"].index(orient) + for _, data, ax in split_gen(): xys = data[["x", "y"]].to_numpy() data = self._resolve_properties(data, scales) - bars = [] + bars, vals = [], [] for i, (x, y) in enumerate(xys): baseline = data["baseline"][i] width = data["width"][i] xy, w, h = coords_to_geometry(x, y, width, baseline) + # Skip bars with no value. It's possible we'll want to make this + # an option (i.e so you have an artist for animating or annotating), + # but let's keep things simple for now. + if not np.nan_to_num(h): + continue + + # TODO Because we are clipping the artist (see below), the edges end up + # looking half as wide as they actually are. I don't love this clumsy + # workaround, which is going to cause surprises if you work with the + # artists directly. We may need to revisit after feedback. + linewidth = data["edgewidth"][i] * 2 + linestyle = data["edgestyle"][i] + if linestyle[1]: + linestyle = (linestyle[0], tuple(x / 2 for x in linestyle[1])) + bar = mpl.patches.Rectangle( xy=xy, width=w, height=h, facecolor=data["facecolor"][i], edgecolor=data["edgecolor"][i], - linewidth=data["edgewidth"][i], - linestyle=data["edgestyle"][i], + linestyle=linestyle, + linewidth=linewidth, + **self.artist_kws, ) + + # This is a bit of a hack to handle the fact that the edge lines are + # centered on the actual extents of the bar, and overlap when bars are + # stacked or dodged. We may discover that this causes problems and needs + # to be revisited at some point. Also it should be faster to clip with + # a bbox than a path, but I cant't work out how to get the intersection + # with the axes bbox. + bar.set_clip_path(bar.get_path(), bar.get_transform() + ax.transData) + if self.artist_kws.get("clip_on", True): + # It seems the above hack undoes the default axes clipping + bar.set_clip_box(ax.bbox) + bar.sticky_edges[val_idx][:] = (0, np.inf) ax.add_patch(bar) bars.append(bar) - - # TODO add container object to ax, line ax.bar does + vals.append(h) + + # Add a container which is useful for, e.g. Axes.bar_label + orientation = {"x": "vertical", "y": "horizontal"}[orient] + container = mpl.container.BarContainer( + bars, datavalues=vals, orientation=orientation, + ) + ax.add_container(container) def _legend_artist( self, variables: list[str], value: Any, scales: dict[str, Scale], diff --git a/tests/_marks/test_bars.py b/tests/_marks/test_bars.py index 1c78022f7a..4f1c6a97ba 100644 --- a/tests/_marks/test_bars.py +++ b/tests/_marks/test_bars.py @@ -76,8 +76,10 @@ def test_direct_properties(self): for bar in ax.patches: assert bar.get_facecolor() == to_rgba(mark.color, mark.alpha) assert bar.get_edgecolor() == to_rgba(mark.edgecolor, mark.edgealpha) - assert bar.get_linewidth() == mark.edgewidth - assert bar.get_linestyle() == (0, mark.edgestyle) + # See comments in plotting method for why we need these adjustments + assert bar.get_linewidth() == mark.edgewidth * 2 + expected_dashes = (mark.edgestyle[0] / 2, mark.edgestyle[1] / 2) + assert bar.get_linestyle() == (0, expected_dashes) def test_mapped_properties(self): From 74854156acfb6053ba3d8b9c53a1e0e9e899e1b1 Mon Sep 17 00:00:00 2001 From: Michael Waskom Date: Tue, 5 Jul 2022 19:56:58 -0400 Subject: [PATCH 2/3] Matplotlib < 3.4 compat --- seaborn/_marks/bars.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/seaborn/_marks/bars.py b/seaborn/_marks/bars.py index 208ff1cd3a..7af42713e0 100644 --- a/seaborn/_marks/bars.py +++ b/seaborn/_marks/bars.py @@ -14,6 +14,7 @@ resolve_properties, resolve_color, ) +from seaborn.external.version import Version from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -124,10 +125,12 @@ def coords_to_geometry(x, y, w, b): vals.append(h) # Add a container which is useful for, e.g. Axes.bar_label - orientation = {"x": "vertical", "y": "horizontal"}[orient] - container = mpl.container.BarContainer( - bars, datavalues=vals, orientation=orientation, - ) + if Version(mpl.__version__) >= Version("3.4.0"): + orientation = {"x": "vertical", "y": "horizontal"}[orient] + container_kws = dict(datavalues=vals, orientation=orientation) + else: + container_kws = {} + container = mpl.container.BarContainer(bars, **container_kws) ax.add_container(container) def _legend_artist( From 5871f833431fd05b28d8bf1714b856b6e06372c9 Mon Sep 17 00:00:00 2001 From: Michael Waskom Date: Tue, 5 Jul 2022 20:08:23 -0400 Subject: [PATCH 3/3] Improve test coverage --- tests/_marks/test_bars.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/_marks/test_bars.py b/tests/_marks/test_bars.py index 4f1c6a97ba..37fdd12d51 100644 --- a/tests/_marks/test_bars.py +++ b/tests/_marks/test_bars.py @@ -92,3 +92,15 @@ def test_mapped_properties(self): assert bar.get_facecolor() == to_rgba(f"C{i}", mark.alpha) assert bar.get_edgecolor() == to_rgba(f"C{i}", 1) assert ax.patches[0].get_linewidth() < ax.patches[1].get_linewidth() + + def test_zero_height_skipped(self): + + p = Plot(["a", "b", "c"], [1, 0, 2]).add(Bar()).plot() + ax = p._figure.axes[0] + assert len(ax.patches) == 2 + + def test_artist_kws_clip(self): + + p = Plot(["a", "b"], [1, 2]).add(Bar({"clip_on": False})).plot() + patch = p._figure.axes[0].patches[0] + assert patch.clipbox is None