Repository navigation
Raise an error when make_subplots padding doesn't fit in its cell - #5826
Open
shivenaggarwal wants to merge 1 commit into
Open
shivenaggarwal wants to merge 1 commit into
shivenaggarwal wants to merge 1 commit into
Conversation
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 plotly#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.
shivenaggarwal
force-pushed
the
subplots-padding-validation
branch
from
October 19, 2026 03:23
b09fb5a to
8a58de1
Compare
Contributor
|
Thanks for the PR! Could you please explain how this PR will help you in your own real-world project or day-to-day work? |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue
Closes #4144
Description of change
The
l/randb/tpadding inmake_subplotsspecsis in normalized plot coordinates. When it adds up to more than the width or height of a subplot's cell, the subplot gets an empty or inverted domain. plotly.js replaces that domain with[0, 1], so the subplot is drawn over the whole figure. In the issue, each of the 8 rows is only ~0.092 tall once the defaultvertical_spacingis taken out, sob+t= 0.1 doesn't fit and all 8 subplots overlap.As suggested in the issue,
make_subplotsnow validates the padding. For each subplot it computes the extent of its cell once, then checksl+randb+tagainst it and applies the padding. This is the same extent the domain is computed from, sorowspan/colspan,row_heights/column_widths,start_celland the spacing are all accounted for. If the padding leaves no room,make_subplotsraises aValueError. Padding that fits gives the same domains as before, and the existing exact-domain tests pass unchanged.Demo
Before: there's no error. Every y domain is inverted, from
[0.9578, 0.95]foryaxisdown to[0.05, 0.0422]foryaxis8. plotly.js replaces all eight with[0, 1](checked withfig.full_figure_for_development()), so the subplots are drawn on top of each other.After:
With
vertical_spacing=0, each cell is 0.125 tall, so the same padding fits and each subplot keeps its own domain.Testing strategy
Added tests to
tests/test_core/test_subplots/test_make_subplots.py:test_make_subplots_padding_errorcoversb/tandl/rpadding larger than the cell, padding that exactly fills the cell, unevenrow_heightswith bothstart_cellvalues,colspan/rowspanpadding larger than the cells spanned, and adomainsubplot. Each case checks the grid position and limit in the message.test_make_subplots_padding_within_cellchecks that padding just under the limit still gives the expected domains. So does padding on spanning subplots that is larger than one cell but smaller than the span.test_make_subplots_padding_issue_4144covers the issue's example, with and withoutvertical_spacing.test_specs_rounding_rounds_down(from #4153) used the example from #4124 (b+t= 0.2 for cells ~0.092 tall). It only checked that no domain went above 1, but those domains were inverted. That input now raises, so the test expects the error and is renamedtest_specs_padding_larger_than_cell.The new error tests fail on
main. These all pass locally:pytest tests/test_coretests/test_optional/test_pxtests/test_optional/test_figure_factoryruff format --check .Additional information (optional)
[0, 1]. A cell with no padding is never flagged, sovertical_spacing=1 / (rows - 1)behaves as before.make_subplotsallows that spacing, even though it leaves the rows empty.[0, 1]. Padding that leaves less than that still passes this check. I kept the rule as "the padding must leave room" rather than copying that constant from plotly.js, but I can add it if you prefer.[0, 1]is unchanged. It is still needed for floating-point noise:make_subplots(rows=8)has a top edge of1.0000000000000002.l/r/t/bare in normalized plot coordinates and must fit in the cell, if you'd like.Guidelines