-
Notifications
You must be signed in to change notification settings - Fork 948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow initial values for model attributes to be specified on creation of the models #383
Allow initial values for model attributes to be specified on creation of the models #383
Conversation
62a2576
to
99ccc93
Compare
99ccc93
to
10eb557
Compare
@jdfreder this seems to complete the blocking issue on persistence for widgets created from the front-end. |
I like the resulting centralization of deserialization here. 👍 |
Nice, this is great cleanup @SylvainCorlay ! |
Allow initial values for model attributes to be specified on creation of the models
🎉 |
@@ -194,7 +172,7 @@ var WidgetModel = Backbone.Model.extend({ | |||
for (var i=0; i<buffer_keys.length; i++) { | |||
state[buffer_keys[i]] = buffers[i]; | |||
} | |||
return that._deserialize_state(state); | |||
return that.constructor._deserialize_state(state, that.manager); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be that.widget_manager
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, see #388
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 👍
introduced in jupyter-widgets#383
attributes
argument so that we can create widget models with initial values._deserialize_state
is now a class method. We need that so that we can deserialize state before instantiating the model, and pass the full deserialized state to the model constructors._deserialize_state
cannot take the model instance as a second argument anymore. And therefore, the same is also true for the custom serializers. Custom deserializers were actually not using this second argument, except the model deserialization (IPY_MODEL_***
to model instance) which in fact only needed a reference to the widget manager. Hence, the second argument passed to_deserialize_state
and the custom serializers is now the widget manager itself._first_state
attribute has been removed from the base model, simplifying our overload of backbone.Model.set.ready
event that was removed since it is not needed anymore. This should solve the last issues with Controller widget persistence.This is a negative contribution in the number of lines, so a good sign!