Update traitlet values before initial render of widget #4956
+38
−35
Conversation
This file contains 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
emilykl
reviewed
Jan 9, 2025
| @@ -729,6 +730,8 @@ def _repr_mimebundle_(self, include=None, exclude=None, validate=True, **kwargs) | |||
| Return mimebundle corresponding to default renderer. | |||
| """ | |||
| display_jupyter_version_warnings() | |||
| self._widget_layout = deepcopy(self._layout_obj._props) | |||
There was a problem hiding this comment.
@marthacryan Can you add a comment explaining why these are here?
emilykl
reviewed
Jan 9, 2025
| if (_.isNil(layout) || _.isNil(layout.width)) { | ||
| // @ts-ignore | ||
| Plotly.Plots.resize(that.el).then(function () { | ||
| var layout_edit_id = that.model.get("_last_layout_edit_id"); | ||
| var layout_edit_id = that.model.get("_last_widget_layout_edit_id"); |
There was a problem hiding this comment.
@marthacryan Shouldn't the name of this traitlet be updated on the Python side as well?
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.
Fixes #4933.
Source of the problem
The issue was that there were updates happening (for example,
add_layout_image) before the initial call to_repr_mimebundle_, which were triggering the traitlet events like_py2js_relayoutor_py2js_addTraces. These events were designed with the idea of a queue that would exist through traitlets, but anywidget (and ipywidgets in general, although for some reason this was working before the anywidget changes) expects the traitlet to be more of a single source of truth for a value, not something that has ordered changes. So by the time that the widget is initialized on the frontend, it loses all of the events that were triggered before the call tofig.show().Solution
This updates the
_repr_mimebundle_method to set the value of the layout and data traitlets before the initial render, so that the changes that were lost in the events queue are still included.This also changes the name of the traitlets for layout and data to
_widget_layoutand_widget_databecause changes were being made to those fields along the way that prevented traitlets from registering a change when they're set in_repr_mimebundle.More info on changing the traitlet names
Some parts of the plotly code change specific properties of the layout traitlets rather than reassigning the entire traitlet. This meant that when we _did_ reassign the value of the traitlets, ipywidgets went to check if there were any changes and ended up deciding that there weren't because it still pointed to the object that had specific fields changed. This seems like a bug but keeping these traitlets separate from the internals of plotly and always reassigning the full value seems like a more straightforward solution.Context on other approaches considered
We considered removing the events system and simplifying the code. See #4945 for what I tried (which did work). However, this would entail a sacrifice in performance because the entire layout or data object would be sent over for any change to the traitlets. This would be much slower than sending only the deltas as we are now.