From 8a58de14db779499a937f630d2e3ecaededffa4b Mon Sep 17 00:00:00 2001 From: Shiven Aggarwal Date: Mon, 19 Oct 2026 08:51:29 +0530 Subject: [PATCH] Raise an error when make_subplots padding doesn't fit in its cell The l/r and b/t padding in the make_subplots specs is in normalized plot coordinates, so it is easy to pass more padding than a cell is wide or tall. For example, 8 rows are only ~0.092 tall each once the default vertical_spacing is taken out, so 0.1 of padding doesn't fit. The subplot then gets an empty or inverted domain, which plotly.js replaces with [0, 1], so each such subplot is drawn over the whole figure and they all overlap. make_subplots now checks the padding of each subplot against the extent of its cell, the same extent its domain is computed from, so rowspan, colspan, row_heights, column_widths and the spacing are taken into account. If the padding leaves no room for the subplot, a ValueError gives the subplot's grid position, the padding values and the cell size they must add up to less than. Padding that fits gives the same domains as before. test_specs_rounding_rounds_down used the example from #4124, whose padding of 0.2 is more than twice the height of its cells, and only checked that no domain went above 1. That input now raises, so the test checks for the error instead. --- CHANGELOG.md | 1 + plotly/_subplots.py | 51 ++++-- .../test_subplots/test_make_subplots.py | 151 +++++++++++++++++- 3 files changed, 185 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c92c3ea6d64..e5205d77d2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Fix Plotly Express mutating lists passed to the `x` or `y` arguments in wide mode [[#5727](https://github.com/plotly/plotly.py/pull/5727)], with thanks to @cpruijsen for the contribution! - Fix `px.sunburst`, `px.treemap` and `px.icicle` listing sectors in a different order on every run when `path` is used with a Polars DataFrame; sectors now follow their order of first appearance for all dataframe backends [[#5766](https://github.com/plotly/plotly.py/pull/5766)], with thanks to @Irahan2 for the contribution! - Fix `mpl_to_plotly` conversion of matplotlib contour lines and line collections: close contour rings ending with `Z` codes, support dash styles, convert date x-axes, separate disjoint subpaths with `None` separators, hide line collection traces from the legend by default, and group consecutive same-style lines into single traces [[#5770](https://github.com/plotly/plotly.py/pull/5770)], with thanks to @robertoffmoura for the contribution! +- Fix `make_subplots` silently drawing subplots over the whole figure when the `l`/`r` or `b`/`t` padding in `specs` doesn't fit in their cells; a `ValueError` naming the subplot and the size of its cell is now raised instead [[#5826](https://github.com/plotly/plotly.py/pull/5826)], with thanks to @shivenaggarwal for the contribution! ## [7.1.0] - 2026-09-15 diff --git a/plotly/_subplots.py b/plotly/_subplots.py index 7983a994a05..2b6e6902228 100644 --- a/plotly/_subplots.py +++ b/plotly/_subplots.py @@ -541,6 +541,30 @@ def _check_hv_spacing(dimsize, spacing, name, dimvarname, dimname): ) ) + def _check_padding(spec, row, col, start, end, cell, dimname): + # Padding that leaves no room for the subplot in its cell would give an + # empty or inverted domain, which plotly.js replaces with [0, 1] + padding = spec[start] + spec[end] + cell_size = cell[1] - cell[0] + # Without padding, a cell can be empty if the spacing is at its maximum + if padding > 0 and padding >= cell_size: + raise ValueError( + """ +The '{start}' and '{end}' padding of the subplot at grid position ({row}, {col}) \ +must add up to less than {cell_size:g}, the {dimname} of its cell in normalized \ +plot coordinates. + Received '{start}': {start_val}, '{end}': {end_val}""".format( + start=start, + end=end, + row=row, + col=col, + cell_size=cell_size, + dimname=dimname, + start_val=spec[start], + end_val=spec[end], + ) + ) + # ### horizontal_spacing ### if horizontal_spacing is None: if has_secondary_y: @@ -681,19 +705,26 @@ def _check_hv_spacing(dimsize, spacing, name, dimvarname, dimname): "Some 'rowspan' value is too large for this subplot grid." ) - # Get x domain using grid and colspan - x_s = grid[r][c][0] + spec["l"] - - x_e = grid[r][c_spanned][0] + widths[c_spanned] - spec["r"] - x_domain = [x_s, x_e] + # Get x extent of the cell using grid and colspan + x_cell = [grid[r][c][0], grid[r][c_spanned][0] + widths[c_spanned]] - # Get y domain (dep. on row_dir) using grid & r_spanned + # Get y extent of the cell (dep. on row_dir) using grid & r_spanned if row_dir > 0: - y_s = grid[r][c][1] + spec["b"] - y_e = grid[r_spanned][c][1] + heights[r_spanned] - spec["t"] + y_cell = [grid[r][c][1], grid[r_spanned][c][1] + heights[r_spanned]] else: - y_s = grid[r_spanned][c][1] + spec["b"] - y_e = grid[r][c][1] + heights[-1 - r] - spec["t"] + y_cell = [grid[r_spanned][c][1], grid[r][c][1] + heights[-1 - r]] + + # Throw exception if the padding leaves no room for the subplot + _check_padding(spec, r + 1, c + 1, "l", "r", x_cell, "width") + _check_padding(spec, r + 1, c + 1, "b", "t", y_cell, "height") + + # Get x and y domain by applying the padding to the cell extent + x_s = x_cell[0] + spec["l"] + x_e = x_cell[1] - spec["r"] + x_domain = [x_s, x_e] + + y_s = y_cell[0] + spec["b"] + y_e = y_cell[1] - spec["t"] if y_s < 0.0: # round for values very close to one diff --git a/tests/test_core/test_subplots/test_make_subplots.py b/tests/test_core/test_subplots/test_make_subplots.py index 52503b0ddef..6be8ec651ba 100644 --- a/tests/test_core/test_subplots/test_make_subplots.py +++ b/tests/test_core/test_subplots/test_make_subplots.py @@ -1,3 +1,4 @@ +import re from unittest import TestCase import pytest from plotly.graph_objs import ( @@ -658,7 +659,10 @@ def test_specs_padding(self): ) self.assertEqual(fig.to_plotly_json(), expected.to_plotly_json()) - def test_specs_rounding_rounds_down(self): + def test_specs_padding_larger_than_cell(self): + # With the default vertical_spacing, each of the 8 cells is ~0.092 tall, + # so 'b' + 't' = 0.2 leaves no room for the subplots. This used to give + # inverted y domains, which plotly.js replaces with [0, 1] n_subplots = 8 padding_size = 0.2 @@ -666,13 +670,8 @@ def test_specs_rounding_rounds_down(self): for _ in range(n_subplots): specs.append([{"b": padding_size / 2.0, "t": padding_size / 2.0}]) - fig = subplots.make_subplots(rows=n_subplots, specs=specs) - self.assertTrue( - all( - fig.layout[f"yaxis{i if i > 1 else ''}"]["domain"][0] <= 1.0 - for i in range(1, n_subplots + 1) - ) - ) + with self.assertRaisesRegex(ValueError, r"less than 0\.0921875, the height"): + subplots.make_subplots(rows=n_subplots, specs=specs) def test_specs_padding_bottom_left(self): expected = Figure( @@ -1961,6 +1960,142 @@ def test_if_passed_figure(self): assert fig2sp.layout == figsp.layout +@pytest.mark.parametrize( + "kwargs, message", + [ + # The cell height doesn't include the vertical_spacing around the cell + pytest.param( + dict(rows=2, vertical_spacing=0.1, specs=[[{}], [{"b": 0.3, "t": 0.2}]]), + "The 'b' and 't' padding of the subplot at grid position (2, 1) " + "must add up to less than 0.45, the height of its cell", + id="b-t", + ), + pytest.param( + dict(cols=2, horizontal_spacing=0.1, specs=[[{}, {"l": 0.25, "r": 0.25}]]), + "The 'l' and 'r' padding of the subplot at grid position (1, 2) " + "must add up to less than 0.45, the width of its cell", + id="l-r", + ), + # Padding that fills the whole cell leaves no room for the subplot + pytest.param( + dict(rows=2, vertical_spacing=0, specs=[[{"b": 0.25, "t": 0.25}], [{}]]), + "grid position (1, 1) must add up to less than 0.5, the height", + id="exact-fit", + ), + # The second row is the 0.2 tall one, whatever the start_cell + pytest.param( + dict( + rows=2, + row_heights=[3, 1], + vertical_spacing=0.2, + specs=[[{"b": 0.15, "t": 0.15}], [{"b": 0.15, "t": 0.15}]], + ), + "grid position (2, 1) must add up to less than 0.2, the height", + id="row_heights", + ), + pytest.param( + dict( + rows=2, + row_heights=[3, 1], + vertical_spacing=0.2, + start_cell="bottom-left", + specs=[[{"b": 0.15, "t": 0.15}], [{"b": 0.15, "t": 0.15}]], + ), + "grid position (2, 1) must add up to less than 0.2, the height", + id="row_heights-bottom-left", + ), + # Spanning subplots can use the cells they span and the spacing between + # them, but no more + pytest.param( + dict( + rows=2, + cols=2, + horizontal_spacing=0.1, + specs=[[{"colspan": 2, "l": 0.6, "r": 0.4}, None], [{}, {}]], + ), + "grid position (1, 1) must add up to less than 1, the width", + id="colspan", + ), + pytest.param( + dict( + rows=3, + vertical_spacing=0.1, + specs=[[{}], [{"rowspan": 2, "b": 0.4, "t": 0.3}], [None]], + ), + "grid position (2, 1) must add up to less than 0.633333, the height", + id="rowspan", + ), + pytest.param( + dict( + rows=3, + vertical_spacing=0.1, + start_cell="bottom-left", + specs=[[{"rowspan": 2, "b": 0.4, "t": 0.3}], [None], [{}]], + ), + "grid position (1, 1) must add up to less than 0.633333, the height", + id="rowspan-bottom-left", + ), + pytest.param( + dict(cols=2, specs=[[{"type": "domain", "l": 0.5}, {}]]), + "grid position (1, 1) must add up to less than 0.45, the width", + id="domain-subplot", + ), + ], +) +def test_make_subplots_padding_error(kwargs, message): + with pytest.raises(ValueError, match=re.escape(message)): + subplots.make_subplots(**kwargs) + + +def test_make_subplots_padding_within_cell(): + # Padding that leaves room for the subplot is applied as before, including + # padding larger than a single cell for subplots spanning several cells + fig = subplots.make_subplots( + rows=3, + cols=2, + horizontal_spacing=0.1, + vertical_spacing=0.05, + specs=[ + [{"rowspan": 2, "b": 0.3, "t": 0.3}, {"l": 0.2, "r": 0.24}], + [None, {"b": 0.15, "t": 0.14}], + [{"colspan": 2, "l": 0.5, "r": 0.4}, None], + ], + ) + # Each cell is 0.45 wide and 0.3 tall + assert fig.get_subplot(1, 1).yaxis.domain == pytest.approx((0.65, 0.7)) + assert fig.get_subplot(1, 2).xaxis.domain == pytest.approx((0.75, 0.76)) + assert fig.get_subplot(2, 2).yaxis.domain == pytest.approx((0.5, 0.51)) + assert fig.get_subplot(3, 1).xaxis.domain == pytest.approx((0.5, 0.6)) + + +def test_make_subplots_padding_issue_4144(): + # With the default vertical_spacing, each of the 8 cells is ~0.092 tall, so + # this padding used to stack all the subplots on top of each other + with pytest.raises( + ValueError, + match=re.escape("must add up to less than 0.0921875, the height of its cell"), + ): + subplots.make_subplots( + rows=8, + cols=1, + row_heights=[0.1] * 8, + specs=[[{"b": 0.05, "t": 0.05}] for _ in range(8)], + ) + + # Without vertical_spacing, each cell is 0.125 tall and the padding fits + fig = subplots.make_subplots( + rows=8, + cols=1, + row_heights=[0.1] * 8, + vertical_spacing=0, + specs=[[{"b": 0.05, "t": 0.05}] for _ in range(8)], + ) + for row in range(1, 9): + cell_top = 1 - 0.125 * (row - 1) + domain = fig.get_subplot(row, 1).yaxis.domain + assert domain == pytest.approx((cell_top - 0.125 + 0.05, cell_top - 0.05)) + + def test_make_subplots_spacing_error(): # check exception describing maximum value for horizontal_spacing or # vertical_spacing is raised when spacing exceeds that value