Merge pull request #2897 from plotly/make_subplots-impossible-spacing

Descriptive error when subplot spacing impossible

Author: nicolaskruchten

Diff for: packages/python/plotly/plotly/express/_core.py

@@ -2120,22 +2120,41 @@ def init_figure(args, subplot_type, frame_list, nrows, ncols, col_labels, row_la
             for j in range(ncols):
                 subplot_labels[i * ncols + j] = col_labels[(nrows - 1 - i) * ncols + j]
 
+    def _spacing_error_translator(e, direction, facet_arg):
+        """
+        Translates the spacing errors thrown by the underlying make_subplots
+        routine into one that describes an argument adjustable through px.
+        """
+        if ("%s spacing" % (direction,)) in e.args[0]:
+            e.args = (
+                e.args[0]
+                + """
+Use the {facet_arg} argument to adjust this spacing.""".format(
+                    facet_arg=facet_arg
+                ),
+            )
+            raise e
+
     # Create figure with subplots
-    fig = make_subplots(
-        rows=nrows,
-        cols=ncols,
-        specs=specs,
-        shared_xaxes="all",
-        shared_yaxes="all",
-        row_titles=[] if facet_col_wrap else list(reversed(row_labels)),
-        column_titles=[] if facet_col_wrap else col_labels,
-        subplot_titles=subplot_labels if facet_col_wrap else [],
-        horizontal_spacing=horizontal_spacing,
-        vertical_spacing=vertical_spacing,
-        row_heights=row_heights,
-        column_widths=column_widths,
-        start_cell="bottom-left",
-    )
+    try:
+        fig = make_subplots(
+            rows=nrows,
+            cols=ncols,
+            specs=specs,
+            shared_xaxes="all",
+            shared_yaxes="all",
+            row_titles=[] if facet_col_wrap else list(reversed(row_labels)),
+            column_titles=[] if facet_col_wrap else col_labels,
+            subplot_titles=subplot_labels if facet_col_wrap else [],
+            horizontal_spacing=horizontal_spacing,
+            vertical_spacing=vertical_spacing,
+            row_heights=row_heights,
+            column_widths=column_widths,
+            start_cell="bottom-left",
+        )
+    except ValueError as e:
+        _spacing_error_translator(e, "Horizontal", "facet_col_spacing")
+        _spacing_error_translator(e, "Vertical", "facet_row_spacing")
 
     # Remove explicit font size of row/col titles so template can take over
     for annot in fig.layout.annotations:

Diff for: packages/python/plotly/plotly/subplots.py

@@ -530,19 +530,41 @@ def _checks(item, defaults):
             )
         )
 
+    def _check_hv_spacing(dimsize, spacing, name, dimvarname, dimname):
+        if spacing < 0 or spacing > 1:
+            raise ValueError("%s spacing must be between 0 and 1." % (name,))
+        if dimsize <= 1:
+            return
+        max_spacing = 1.0 / float(dimsize - 1)
+        if spacing > max_spacing:
+            raise ValueError(
+                """{name} spacing cannot be greater than (1 / ({dimvarname} - 1)) = {max_spacing:f}.
+The resulting plot would have {dimsize} {dimname} ({dimvarname}={dimsize}).""".format(
+                    dimvarname=dimvarname,
+                    name=name,
+                    dimname=dimname,
+                    max_spacing=max_spacing,
+                    dimsize=dimsize,
+                )
+            )
+
     # ### horizontal_spacing ###
     if horizontal_spacing is None:
         if has_secondary_y:
             horizontal_spacing = 0.4 / cols
         else:
             horizontal_spacing = 0.2 / cols
+    # check horizontal_spacing can be satisfied:
+    _check_hv_spacing(cols, horizontal_spacing, "Horizontal", "cols", "columns")
 
     # ### vertical_spacing ###
     if vertical_spacing is None:
         if subplot_titles is not None:
             vertical_spacing = 0.5 / rows
         else:
             vertical_spacing = 0.3 / rows
+    # check vertical_spacing can be satisfied:
+    _check_hv_spacing(rows, vertical_spacing, "Vertical", "rows", "rows")
 
     # ### subplot titles ###
     if subplot_titles is None:

Diff for: packages/python/plotly/plotly/tests/test_core/test_px/test_facets.py

@@ -1,5 +1,9 @@
+import plotly
+import pandas as pd
 import plotly.express as px
 from pytest import approx
+import pytest
+import random
 
 
 def test_facets():
@@ -41,3 +45,45 @@ def test_facets():
     )
     assert fig.layout.xaxis4.domain[0] - fig.layout.xaxis.domain[1] == approx(0.09)
     assert fig.layout.yaxis4.domain[0] - fig.layout.yaxis.domain[1] == approx(0.08)
+
+
+@pytest.fixture
+def bad_facet_spacing_df():
+    NROWS = 101
+    NDATA = 1000
+    categories = [n % NROWS for n in range(NDATA)]
+    df = pd.DataFrame(
+        {
+            "x": [random.random() for _ in range(NDATA)],
+            "y": [random.random() for _ in range(NDATA)],
+            "category": categories,
+        }
+    )
+    return df
+
+
+def test_bad_facet_spacing_eror(bad_facet_spacing_df):
+    df = bad_facet_spacing_df
+    with pytest.raises(
+        ValueError, match="Use the facet_row_spacing argument to adjust this spacing\."
+    ):
+        fig = px.scatter(
+            df, x="x", y="y", facet_row="category", facet_row_spacing=0.01001
+        )
+    with pytest.raises(
+        ValueError, match="Use the facet_col_spacing argument to adjust this spacing\."
+    ):
+        fig = px.scatter(
+            df, x="x", y="y", facet_col="category", facet_col_spacing=0.01001
+        )
+    # Check error is not raised when the spacing is OK
+    try:
+        fig = px.scatter(df, x="x", y="y", facet_row="category", facet_row_spacing=0.01)
+    except ValueError:
+        # Error shouldn't be raised, so fail if it is
+        assert False
+    try:
+        fig = px.scatter(df, x="x", y="y", facet_col="category", facet_col_spacing=0.01)
+    except ValueError:
+        # Error shouldn't be raised, so fail if it is
+        assert False

Diff for: packages/python/plotly/plotly/tests/test_core/test_subplots/test_make_subplots.py

@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 
 from unittest import TestCase
+import pytest
 from plotly.graph_objs import (
     Annotation,
     Annotations,
@@ -1934,3 +1935,80 @@ def test_if_passed_figure(self):
         fig2sp = subplots.make_subplots(2, 2)
         assert id(fig2sp) != id(figsp)
         assert fig2sp.layout == figsp.layout
+
+
+def test_make_subplots_spacing_error():
+    # check exception describing maximum value for horizontal_spacing or
+    # vertical_spacing is raised when spacing exceeds that value
+    for match in [
+        (
+            "^%s spacing cannot be greater than \(1 / \(%s - 1\)\) = %f."
+            % ("Vertical", "rows", 1.0 / 50.0)
+        ).replace(".", "\."),
+        "The resulting plot would have 51 rows \(rows=51\)\.$",
+    ]:
+        with pytest.raises(
+            ValueError, match=match,
+        ):
+            fig = subplots.make_subplots(51, 1, vertical_spacing=0.0201)
+    for match in [
+        (
+            "^%s spacing cannot be greater than \(1 / \(%s - 1\)\) = %f."
+            % ("Horizontal", "cols", 1.0 / 50.0)
+        ).replace(".", "\."),
+        "The resulting plot would have 51 columns \(cols=51\)\.$",
+    ]:
+        with pytest.raises(
+            ValueError, match=match,
+        ):
+            fig = subplots.make_subplots(1, 51, horizontal_spacing=0.0201)
+    # Check it's not raised when it's not beyond the maximum
+    try:
+        fig = subplots.make_subplots(51, 1, vertical_spacing=0.0200)
+    except ValueError:
+        # This shouldn't happen so we assert False to force failure
+        assert False
+    try:
+        fig = subplots.make_subplots(1, 51, horizontal_spacing=0.0200)
+    except ValueError:
+        # This shouldn't happen so we assert False to force failure
+        assert False
+    # make sure any value between 0 and 1 works for horizontal_spacing if cols is 1
+    try:
+        fig = subplots.make_subplots(1, 1, horizontal_spacing=0)
+    except ValueError:
+        # This shouldn't happen so we assert False to force failure
+        assert False
+    try:
+        fig = subplots.make_subplots(1, 1, horizontal_spacing=1)
+    except ValueError:
+        # This shouldn't happen so we assert False to force failure
+        assert False
+    # make sure any value between 0 and 1 works for horizontal_spacing if cols is 1
+    try:
+        fig = subplots.make_subplots(1, 1, horizontal_spacing=0)
+    except ValueError:
+        # This shouldn't happen so we assert False to force failure
+        assert False
+    # make sure any value between 0 and 1 works for horizontal_spacing if cols is 1
+    try:
+        fig = subplots.make_subplots(1, 1, horizontal_spacing=1)
+    except ValueError:
+        # This shouldn't happen so we assert False to force failure
+        assert False
+    with pytest.raises(
+        ValueError, match="^Horizontal spacing must be between 0 and 1\.$"
+    ):
+        fig = subplots.make_subplots(1, 1, horizontal_spacing=-0.01)
+    with pytest.raises(
+        ValueError, match="^Horizontal spacing must be between 0 and 1\.$"
+    ):
+        fig = subplots.make_subplots(1, 1, horizontal_spacing=1.01)
+    with pytest.raises(
+        ValueError, match="^Vertical spacing must be between 0 and 1\.$"
+    ):
+        fig = subplots.make_subplots(1, 1, vertical_spacing=-0.01)
+    with pytest.raises(
+        ValueError, match="^Vertical spacing must be between 0 and 1\.$"
+    ):
+        fig = subplots.make_subplots(1, 1, vertical_spacing=1.01)