Repository navigation
Fix row and column titles in make_subplots when edge cells are spanned - #5793
joaopedroassad wants to merge 2 commits into
Conversation
843b8e1 to
bc7ae12
Compare
chrikrah
left a comment
There was a problem hiding this comment.
@joaopedroassad Approving: the new test fails on main, and a wider sweep across uneven heights and widths found no title that moves. The one thing left is a CHANGELOG.md conflict with main; the code needs nothing.
$ pytest -q tests/test_core/test_subplots # at bc7ae12
72 passed, 403 warnings in 1.80s
$ pytest -q tests/test_core/test_subplots # base 9447f1c
69 passed, 403 warnings in 1.74s
$ cp <base>/plotly/_subplots.py plotly/ && pytest -q tests/test_core/test_subplots -k spanning
3 failed, 69 deselected in 0.51s
non-blocking: the three parametrized layouts all use equal rows and columns. I compared spanned and plain grids for annotation text, x and y on three more layouts (3x3 with both spans, a full-height rowspan, a full-width colspan), crossed with both start_cell values, row_heights=[1, 2, 3] and column_widths=[3, 1, 2]. The branch gives 0 of 24 mismatches, main gives 20 of 24. The legacy row_width order matches too. One uneven case in the test would pin the heights[-1 - r] branch in _title_domain.
# not run: tests/test_optional, the rest of tests/test_core
@joaopedroassad would you add one uneven row_heights case to the parametrization? Happy to send the layouts I used.
bc7ae12 to
5ed69f2
Compare
|
Thanks @chrikrah. I added two uneven cases to the parametrization: In I also rebased onto main to clear the CHANGELOG.md conflict. |
Link to issue
Closes #5792
Description of change
make_subplotsnow keeps every row and column title aligned with its own row/column when the edge cell it is anchored to is empty or covered by a subplot withrowspan/colspan. Before, that cell was skipped, which pushed the remaining titles onto the wrong rows/columns and dropped the last one.Demo
Before:
[('Row A', 0.212)](Row A next to row 2, Row B missing)After:
[('Row A', 0.787), ('Row B', 0.212)]Testing strategy
Added
test_row_and_column_titles_with_spanning_subplots, parametrized over three layouts (colspan covering the last cell of a row, rowspan in the last column, and a rowspan withstart_cell="bottom-left"that leaves a top-row cell empty). It checks that the title positions match the same grid without spans. It fails on main and passes with this change. The existing subplot tests still pass, and regular grids (padding, uneven widths/heights, shared axes, secondary_y) produce exactly the same annotations as before.Additional information (optional)
When the cell does hold a subplot that covers exactly that row/column, its own domain is still used, so
l/r/t/bpadding behaves as it did before. Only empty or spanned cells fall back to the grid cell geometry.Guidelines