Improve visual appearance of Bar mark (#2889)

* Improve visual appearance of Bar mark

* Matplotlib < 3.4 compat

* Improve test coverage

Author: mwaskom

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 (
@@ -13,6 +14,7 @@
     resolve_properties,
     resolve_color,
 )
+from seaborn.external.version import Version
 
 from typing import TYPE_CHECKING
 if TYPE_CHECKING:
@@ -67,31 +69,69 @@ 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)
+                vals.append(h)
 
-            # TODO add container object to ax, line ax.bar does
+            # Add a container which is useful for, e.g. Axes.bar_label
+            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(
         self, variables: list[str], value: Any, scales: dict[str, Scale],

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):
 
@@ -90,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