Improve user feedback for errors during plot compilation (#3203)

* Improve user feedback for errors during plot compilation

* Update release notes and fix flaky test

* Fix pytest.raises usage and improve tests

* Simplify comments for cleaner tracebacks

doc/whatsnew/v0.12.2.rst

@@ -6,9 +6,11 @@ v0.12.2 (Unreleased)
 
 - |Enhancement| Automatic mark widths are now calculated separately for unshared facet axes (:pr:`3119`).
 
+- |Enhancement| Improved user feedback for failures during plot compilation by catching exceptions an reraising with a `PlotSpecError` that provides additional context (:pr:`3203`).
+
 - |Fix| Fixed a bug where legends for numeric variables with large values with be incorrectly shown (i.e. with a missing offset or exponent; :pr:`3187`).
 
-- |Fix| Improve robustness to empty data in several components of the objects interface (:pr:`3202`).
+- |Fix| Improved robustness to empty data in several components of the objects interface (:pr:`3202`).
 
 - |Fix| Fixed a regression in v0.12.0 where manually-added labels could have duplicate legend entries (:pr:`3116`).
 

seaborn/_core/exceptions.py

@@ -0,0 +1,32 @@
+"""
+Custom exceptions for the seaborn.objects interface.
+
+This is very lightweight, but it's a separate module to avoid circular imports.
+
+"""
+from __future__ import annotations
+
+
+class PlotSpecError(RuntimeError):
+ """
+ Error class raised from seaborn.objects.Plot for compile-time failures.
+
+ In the declarative Plot interface, exceptions may not be triggered immediately
+ by bad user input (and validation at input time may not be possible). This class
+ is used to signal that indirect dependency. It should be raised in an exception
+ chain when compile-time operations fail with an error message providing useful
+ context (e.g., scaling errors could specify the variable that failed.)
+
+ """
+ @classmethod
+ def _during(cls, step: str, var: str = "") -> PlotSpecError:
+ """
+ Initialize the class to report the failure of a specific operation.
+ """
+ message = []
+ if var:
+ message.append(f"{step} failed for the `{var}` variable.")
+ else:
+ message.append(f"{step} failed.")
+ message.append("See the traceback above for more information.")
+ return cls(" ".join(message))

seaborn/_core/plot.py

@@ -36,6 +36,7 @@
  OrderSpec,
  Default,
 )
+from seaborn._core.exceptions import PlotSpecError
 from seaborn._core.rules import categorical_order
 from seaborn._compat import set_scale_obj, set_layout_engine
 from seaborn.rcmod import axes_style, plotting_context
@@ -1249,14 +1250,13 @@ def _setup_scales(
  if scale is None:
  self._scales[var] = Scale._identity()
  else:
- self._scales[var] = scale._setup(var_df[var], prop)
+ try:
+ self._scales[var] = scale._setup(var_df[var], prop)
+ except Exception as err:
+ raise PlotSpecError._during("Scale setup", var) from err
 
- # Everything below here applies only to coordinate variables
- # We additionally skip it when we're working with a value
- # that is derived from a coordinate we've already processed.
- # e.g., the Stat consumed y and added ymin/ymax. In that case,
- # we've already setup the y scale and ymin/max are in scale space.
  if axis is None or (var != coord and coord in p._variables):
+ # Everything below here applies only to coordinate variables
  continue
 
  # Set up an empty series to receive the transformed values.
@@ -1276,9 +1276,15 @@ def _setup_scales(
 
  for layer, new_series in zip(layers, transformed_data):
  layer_df = layer["data"].frame
- if var in layer_df:
- idx = self._get_subplot_index(layer_df, view)
+ if var not in layer_df:
+ continue
+
+ idx = self._get_subplot_index(layer_df, view)
+ try:
  new_series.loc[idx] = view_scale(layer_df.loc[idx, var])
+ except Exception as err:
+ spec_error = PlotSpecError._during("Scaling operation", var)
+ raise spec_error from err
 
  # Now the transformed data series are complete, set update the layer data
  for layer, new_series in zip(layers, transformed_data):

seaborn/_marks/base.py

@@ -20,6 +20,7 @@
  DashPattern,
  DashPatternWithOffset,
 )
+from seaborn._core.exceptions import PlotSpecError
 
 
 class Mappable:
@@ -172,7 +173,13 @@ def _resolve(
  # TODO Might this obviate the identity scale? Just don't add a scale?
  feature = data[name]
  else:
- feature = scales[name](data[name])
+ scale = scales[name]
+ value = data[name]
+ try:
+ feature = scale(value)
+ except Exception as err:
+ raise PlotSpecError._during("Scaling operation", name) from err
+
  if return_array:
  feature = np.asarray(feature)
  return feature

seaborn/palettes.py

@@ -234,7 +234,7 @@ def color_palette(palette=None, n_colors=None, desat=None, as_cmap=False):
  # Perhaps a named matplotlib colormap?
  palette = mpl_palette(palette, n_colors, as_cmap=as_cmap)
  except (ValueError, KeyError): # Error class changed in mpl36
- raise ValueError(f"{palette} is not a valid palette name")
+ raise ValueError(f"{palette!r} is not a valid palette name")
 
  if desat is not None:
  palette = [desaturate(c, desat) for c in palette]

tests/_core/test_plot.py

@@ -15,12 +15,14 @@
 from numpy.testing import assert_array_equal, assert_array_almost_equal
 
 from seaborn._core.plot import Plot, Default
-from seaborn._core.scales import Nominal, Continuous
-from seaborn._core.rules import categorical_order
+from seaborn._core.scales import Continuous, Nominal, Temporal
 from seaborn._core.moves import Move, Shift, Dodge
-from seaborn._stats.aggregation import Agg
+from seaborn._core.rules import categorical_order
+from seaborn._core.exceptions import PlotSpecError
 from seaborn._marks.base import Mark
 from seaborn._stats.base import Stat
+from seaborn._marks.dot import Dot
+from seaborn._stats.aggregation import Agg
 from seaborn.external.version import Version
 
 assert_vector_equal = functools.partial(
@@ -1249,6 +1251,54 @@ def test_title_facet_function(self):
  assert ax.get_title() == expected
 
 
+class TestExceptions:
+
+ def test_scale_setup(self):
+
+ x = y = color = ["a", "b"]
+ bad_palette = "not_a_palette"
+ p = Plot(x, y, color=color).add(MockMark()).scale(color=bad_palette)
+
+ msg = "Scale setup failed for the `color` variable."
+ with pytest.raises(PlotSpecError, match=msg) as err:
+ p.plot()
+ assert isinstance(err.value.__cause__, ValueError)
+ assert bad_palette in str(err.value.__cause__)
+
+ def test_coordinate_scaling(self):
+
+ x = ["a", "b"]
+ y = [1, 2]
+ p = Plot(x, y).add(MockMark()).scale(x=Temporal())
+
+ msg = "Scaling operation failed for the `x` variable."
+ with pytest.raises(PlotSpecError, match=msg) as err:
+ p.plot()
+ # Don't test the cause contents b/c matplotlib owns them here.
+ assert hasattr(err.value, "__cause__")
+
+ def test_semantic_scaling(self):
+
+ class ErrorRaising(Continuous):
+
+ def _setup(self, data, prop, axis=None):
+
+ def f(x):
+ raise ValueError("This is a test")
+
+ new = super()._setup(data, prop, axis)
+ new._pipeline = [f]
+ return new
+
+ x = y = color = [1, 2]
+ p = Plot(x, y, color=color).add(Dot()).scale(color=ErrorRaising())
+ msg = "Scaling operation failed for the `color` variable."
+ with pytest.raises(PlotSpecError, match=msg) as err:
+ p.plot()
+ assert isinstance(err.value.__cause__, ValueError)
+ assert str(err.value.__cause__) == "This is a test"
+
+
 class TestFacetInterface:
 
  @pytest.fixture(scope="class", params=["row", "col"])

tests/_core/test_scales.py

@@ -448,7 +448,7 @@ def test_color_alpha_in_palette(self, x):
  def test_color_unknown_palette(self, x):
 
  pal = "not_a_palette"
- err = f"{pal} is not a valid palette name"
+ err = f"'{pal}' is not a valid palette name"
  with pytest.raises(ValueError, match=err):
  Nominal(pal)._setup(x, Color())
 

tests/test_distributions.py

@@ -1934,8 +1934,8 @@ def test_mesh_log_scale(self, rng):
  edges = itertools.product(y_edges[:-1], x_edges[:-1])
  for i, (y_i, x_i) in enumerate(edges):
  path = mesh.get_paths()[i]
- assert path.vertices[0, 0] == 10 ** x_i
- assert path.vertices[0, 1] == 10 ** y_i
+ assert path.vertices[0, 0] == pytest.approx(10 ** x_i)
+ assert path.vertices[0, 1] == pytest.approx(10 ** y_i)
 
  def test_mesh_thresh(self, long_df):