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