Skip to content

Commit

Permalink
Rename the widget model .id attribute to .model_id
Browse files Browse the repository at this point in the history
In Backbone, it will set the .id attribute automatically based on model data. This also may mean that it will make the .id attribute undefined after we have set it.
  • Loading branch information
jasongrout committed Jun 24, 2017
1 parent b3e0c81 commit cff5f5a
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 21 deletions.
2 changes: 1 addition & 1 deletion jupyter-widgets-base/src/manager-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ abstract class ManagerBase<T> {
});
view.listenTo(model, 'destroy', view.remove);
return Promise.resolve(view.render()).then(() => {return view;});
}).catch(utils.reject('Could not create a view for model id ' + model.id, true));
}).catch(utils.reject('Could not create a view for model id ' + model.model_id, true));
});
let id = utils.uuid();
model.views[id] = viewPromise;
Expand Down
7 changes: 3 additions & 4 deletions jupyter-widgets-base/src/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class WidgetModel extends Backbone.Model {
super.initialize(attributes, options);

this.widget_manager = options.widget_manager;
this.id = options.model_id;
this.model_id = options.model_id;
let comm = options.comm;

// _buffered_state_diff must be created *after* the super.initialize
Expand Down Expand Up @@ -186,7 +186,7 @@ class WidgetModel extends Backbone.Model {
return (this.constructor as typeof WidgetModel)._deserialize_state(state, this.widget_manager);
}).then((state) => {
this.set_state(state);
}).catch(utils.reject(`Could not process update msg for model id: ${this.id}`, true))
}).catch(utils.reject(`Could not process update msg for model id: ${this.model_id}`, true))
return this.state_change;
case 'custom':
this.trigger('msg:custom', msg.content.data.content, msg.buffers);
Expand Down Expand Up @@ -467,7 +467,7 @@ class WidgetModel extends Backbone.Model {
* and the kernel-side serializer/deserializer.
*/
toJSON(options) {
return `IPY_MODEL_${this.id}`;
return `IPY_MODEL_${this.model_id}`;
}

/**
Expand Down Expand Up @@ -686,7 +686,6 @@ class DOMWidgetView extends WidgetView {
*/
initialize(parameters) {
super.initialize(parameters);
this.id = utils.uuid();

this.listenTo(this.model, 'change:_dom_classes', (model, new_classes) => {
let old_classes = model.previous('_dom_classes');
Expand Down
16 changes: 8 additions & 8 deletions jupyter-widgets-base/test/src/manager_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe("ManagerBase", function() {
it('returns a promise to the model', async function() {
let manager = this.managerBase
let model = await manager.new_model(this.modelOptions);
expect(await manager.get_model(model.id)).to.be.equal(model);
expect(await manager.get_model(model.model_id)).to.be.equal(model);
});

it('returns undefined when model is not registered', function() {
Expand Down Expand Up @@ -217,7 +217,7 @@ describe("ManagerBase", function() {
let manager = new NewWidgetManager();
let model = await manager.new_widget(spec);
expect(model.comm).to.be.undefined;
expect(model.id).to.not.be.undefined;
expect(model.model_id).to.not.be.undefined;
});
});

Expand All @@ -242,7 +242,7 @@ describe("ManagerBase", function() {
};
let manager = this.managerBase;
let model = await manager.new_model(spec);
expect(model.id).to.be.equal(comm.comm_id);
expect(model.model_id).to.be.equal(comm.comm_id);
});

it('rejects if model_id or comm not given', async function() {
Expand Down Expand Up @@ -304,7 +304,7 @@ describe("ManagerBase", function() {
let manager = this.managerBase;
let model = await manager.new_model(spec);
comm.close();
expect(manager.get_model(model.id)).to.be.undefined;
expect(manager.get_model(model.model_id)).to.be.undefined;
});
});

Expand All @@ -324,11 +324,11 @@ describe("ManagerBase", function() {
let manager = this.managerBase;
let model1 = await manager.new_model(mSpec1);
let model2 = await manager.new_model(mSpec2);
expect(await manager.get_model(model1.id)).to.be.equal(model1);
expect(await manager.get_model(model2.id)).to.be.equal(model2);
expect(await manager.get_model(model1.model_id)).to.be.equal(model1);
expect(await manager.get_model(model2.model_id)).to.be.equal(model2);
await manager.clear_state();
expect(manager.get_model(model1.id)).to.be.undefined;
expect(manager.get_model(model2.id)).to.be.undefined;
expect(manager.get_model(model1.model_id)).to.be.undefined;
expect(manager.get_model(model2.model_id)).to.be.undefined;
expect((comm1.close as any).calledOnce).to.be.true;
expect((comm2.close as any).calledOnce).to.be.true;
expect(model1.comm).to.be.undefined;
Expand Down
6 changes: 3 additions & 3 deletions jupyter-widgets-base/test/src/widget_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe("WidgetModel", function() {
model_id: 'widgetDead',
widget_manager: this.manager,
});
expect(widgetDead.id).to.equal('widgetDead');
expect(widgetDead.model_id).to.equal('widgetDead');
expect(widgetDead.widget_manager).to.equal(this.manager);
expect(widgetDead.comm).to.be.undefined;
expect(widgetDead.comm_live).to.be.false;
Expand All @@ -133,7 +133,7 @@ describe("WidgetModel", function() {
widget_manager: this.manager,
comm: comm
});
expect(widgetLive.id).to.equal('widgetLive');
expect(widgetLive.model_id).to.equal('widgetLive');
expect(widgetLive.widget_manager).to.equal(this.manager);
expect(widgetLive.comm).to.equal(comm);
expect(widgetLive.comm_live).to.be.true;
Expand Down Expand Up @@ -702,7 +702,7 @@ describe("WidgetModel", function() {
});

it('encodes the widget', function() {
expect(this.widget.toJSON()).to.equal(`IPY_MODEL_${this.widget.id}`);
expect(this.widget.toJSON()).to.equal(`IPY_MODEL_${this.widget.model_id}`);
});
});
});
2 changes: 1 addition & 1 deletion jupyter-widgets-htmlmanager/src/embed-webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function renderManager(element, tag) {
}
let model_id = widgetViewObject.model_id;
let model = _.find(models, function(item : WidgetModel) {
return item.id == model_id;
return item.model_id == model_id;
});
if (model !== undefined) {
if (viewtag.previousElementSibling &&
Expand Down
8 changes: 4 additions & 4 deletions widgetsnbextension/src/widget_output.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var OutputModel = widgets.DOMWidgetModel.extend({
this.kernel = this.comm.kernel;
this.listenTo(this, 'change:msg_id', this.reset_msg_id);
if (this.kernel) {
this.kernel.set_callbacks_for_msg(this.id, this.callbacks(), false);
this.kernel.set_callbacks_for_msg(this.model_id, this.callbacks(), false);
}

var that = this;
Expand Down Expand Up @@ -80,13 +80,13 @@ var OutputModel = widgets.DOMWidgetModel.extend({
var prev_msg_id = this.previous('msg_id');
if (prev_msg_id && kernel) {
var previous_callback = kernel.output_callback_overrides_pop(prev_msg_id);
if (previous_callback !== this.id) {
console.error('Popped wrong message ('+previous_callback+' instead of '+this.id+') - likely the stack was not maintained in kernel.');
if (previous_callback !== this.model_id) {
console.error('Popped wrong message ('+previous_callback+' instead of '+this.model_id+') - likely the stack was not maintained in kernel.');
}
}
var msg_id = this.get('msg_id');
if (msg_id && kernel) {
kernel.output_callback_overrides_push(msg_id, this.id);
kernel.output_callback_overrides_push(msg_id, this.model_id);
}
},

Expand Down

0 comments on commit cff5f5a

Please sign in to comment.