Don't leave empty dict containers behind when nested properties are set to None#5637
Open
ahmojo wants to merge 1 commit into
Open
Don't leave empty dict containers behind when nested properties are set to None#5637ahmojo wants to merge 1 commit into
ahmojo wants to merge 1 commit into
Conversation
…et to None
Setting a nested property such as marker.colorbar.thicknessmode to None
left a residual empty {} container in the figure's props, which was then
emitted in the figure JSON. For splom traces this residual
marker.colorbar {} made a later Plotly.restyle of colorbar attributes
misconvert the colorbar thickness and collapse the scatter-matrix layout.
_set_in now treats removal of a non-existent path as a no-op and prunes
emptied dict parents, and property assignment to None prunes emptied
compound-child dicts (lists are preserved as positional placeholders).
Fixes plotly#5615
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
1 participant
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 #5615
Description of change
Setting a nested property to
None(e.g.fig.data[i].marker.colorbar.thicknessmode = None, orplotly_restyle({"marker.colorbar.thicknessmode": None})) left a residual empty{}container in the emitted figure JSON. For splom traces, that residualmarker: {colorbar: {}}causes a laterPlotly.restyleof colorbar attributes to run plotly.js's thicknessmode fraction→pixels conversion and write a huge thickness (30 × plot width, e.g. 16290px) into the trace, collapsing the scatter-matrix layout. This PR makes removal of a non-existent path a no-op and prunes emptied{}containers, so the figure JSON only contains properties the user actually set.Demo
I isolated the trigger with the issue's Playwright repro (1200×800 viewport, bundled plotly.js):
colorbarkeycolorbar: {}+ JS restyle (the issue){"colorbar": {}}colorbarkeyA pristine figure never collapses — only the residual empty dict emitted by plotly.py triggers it. With this fix, the issue's exact script renders normally.
Testing strategy
Added two regression tests in
tests/test_core/test_figure_messages/test_plotly_restyle.py: one asserting thatplotly_restyle(..., None)on a never-set path leaves the figure unchanged and sends no restyle message, and one asserting that unsetting a nested attribute leaves no empty{}residue in the emitted JSON. Both fail onmainand pass with this change;tests/test_corepasses.Additional information (optional)
Lists are intentionally preserved as positional placeholders (e.g.
dimensions[1]keeps its index), and pruning is skipped in batch mode until the batch flushes. Arguably plotly.js also shouldn't run the fraction→pixels thickness conversion for an empty pre-existing colorbar container when the default mode is alreadypixels— happy to file that upstream as a follow-up if useful.Guidelines