IPEP 23: Backbone.js Widgets #4374
Conversation
|
Uploaded some widgets to play with, |
|
PR description updated with some small examples and an index of the widget/view pairs was added. |
|
Made changes discussed in meeting and added ToggleButtonView for the SelectionWidget. |
|
NOTE: JS test will not pass until jupyter/ipython-components#10 is merged |
|
Continuing a discussion from HipChat, @cekees This PR provides an MVC-like architecture on top of the comm stuff. It's useful for widgets that can be expressed via a list of properties. This PR synchronizes a python traitlet model in the backend with a backbone model in the front. Some of the things it automatically handles are
EDIT 11/20/2013: Support for custom messages was added a couple of days ago. |
|
Sorry; I haven't had time to look at the code in this pull request much, but I'm curious: how much does this depend on the architecture of the notebook? I'd like to see about adapting this to the Sage cell server, but we don't have the same concept of 'cells' as in the notebook. It seems like (in principle) this could be more general than depending on details of the IPython notebook front end. If your answer is: "Read the code, tell us what you think", that's fine and understandable. I apologize for not having time (yet) to go through this carefully. |
|
Hi @jasongrout , I think I don't fully understand the context of where you want to implement this code. If you are planning on using it within a modified IPython environment where only one cell exists, that should work great (as long as the comm stuff is available). If you are planning on using it elsewhere, the coupling with IPython isn't horrible, but it will require a little bit of work. As of now it depends on
EDIT 11/4/13: Updated with new details |
|
With regards to the msg_id mapping, see this JS function https://github.com/ipython/ipython/pull/4374/files#diff-309e41632e4bd9e130ed388abdd89575R294 . This is fairly recent code and not the only place where the mapping is (also the function that makes the comm callback dict). I think I'm going to try to do a better job of reducing the coupling with cell_index. |
|
@jasongrout I just made changes that decoupled the cell_index from the mapping code. Now everything that you would need to modify for custom mapping to use the code outside of IPython is contained in the EDIT: 11/4/13: Added |
|
|
|
Thanks. Encapsulating all the IPython notebook-specific things, like what the output DOM looks like, into a function or two that I could override is what I was hoping for. If I understand things correctly, there is only one widget area per cell? That seems kind of limiting. I'd love to print some text, have a widget, print some more text, have another widget, etc. Is there any way to embed widgets in the normal output flow? I mean, I can embed images, audio, and all sorts of other things in the normal output region of a cell, inline with the other output---why not widgets too? |
I totally agree with you. Ideally you would be able to place widgets wherever you want. This is one of the design questions that we talked about in one of the first dev meetings regarding this subject. @ellisonbg and I re-discussed the issue when I started implementing this. The problem is that we don't want the widgets to be cleared along with the output. In other words, we want the user to be able to make a widget and hookup the value change event to a function that clears the output and plots. If the output clear event cleared the entire output with the widgets included, the widget on the front end and corresponding com would get closed. Then the user would have to re-show the widget with the output. Even if the clear output selectively cleared the output, the placement of the widgets would be ruined. |
|
@jasongrout I fixed the bug, and updated my comments above with the changes. |
|
@jasongrout Also with the new changes, you could implement multiple widget areas by just overriding |
|
Sorry I haven't kept up on the design discussions. In the case you mentioned, couldn't the widget just be hooked up to an "output widget" containing the plot, and send the clear message just to that output widget? We've done similar things for Sage interacts, and it seems to work well (nested interacts, for example, like http://sagecell.sagemath.org/?q=zkjyys -- change the numbers of rows and columns, and the inner interact updates accordingly). It nicely encapsulates the effects of a widget too. In fact, I suppose you could implement the standard output of a cell as an "output" widget itself, and have turtles all the way down. |
It could, but in that case why bother with interweaving the output area and widgets at all? The existing framework can behave like that already. There is already a LabelView (which lets you display arbitrary HTML in the widget area). The only thing that is missing is an ImageWidget or PlotWidget. Maybe that's something that should be included by default? I think someone else is working on porting the matplot image data over a comm (unfortunately I don't know where that is being done). An image widget was on my todo list at one point, but @ellisonbg and I removed it. |
|
That was me working on porting matplotlib images over to the comm framework :). I had to leave it for the time being, but hope to pick it back up before or during the holiday season. There's also a generic "output widget" missing, right? Suppose I want to execute a user function every time a button is clicked, and put whatever output the user did in that function in a box below the button (this is similar to how Sage interacts work right now). But back to the other discussion---I'm just seeing if there is a way to combine the widget and general output regions. The point is not whether I can interleave widgets and output, but whether that should be the default. In the use-case you mentioned above, to me it seems more elegant to make the plot that would be cleared a specific region in the output, like this generic "output widget", that could be cleared independent of the rest of the output. Also, how would I actually clear/destroy the widget area? |
Awesome! Yeah I can't wait till that works. I think that would be much better than having a plot or image widget that tries to transmit all of that data through state.
Maybe, I think redirecting output from the standard output area to a widget is not possible with the current callback system. I believe that would require the msg callbacks stored in the front-end (that direct output to the correct cell) to be modified in the middle of a cell execution in the back-end. I don't think there is a message that allows this to be done. @minrk would definitely know how to do this and if it could be done.
It may be more elegant, but I don't think it would be the simplest solution. Right now a user can create a widget and register a callback for it's value change. In that callback the user clears the output and draws a plot, just like he/she normally would for simple animations. The benefit is that the output area is predictable and behaves like it does without widgets. Also, just to make sure we are on the same page, these widgets are not a form of interact. Interact can and will be built on top of these widgets, which is what @ellisonbg has shown and is still working on. |
|
I've running this branch without any problems on my local installation, even doing a little bit of UI development for some computational models using the IPython widgets, but I can't seem to get them to work when I run ipython notebook from a sage math cloud instances. No errors, but also no widgets. @jasongrout have you tried these on SMC? Perhaps interestingly, @minrk's low level comm example with the range widget works perfectly. The IPython commit ID is 2cc3475. |
|
@cekees is it because jupyter/ipython-components#10 isn't checked out on the sage server too? |
|
The JavaScript console in your web browser should mention missing backbone.js and underscore.js |
|
Ah! It looks like the path to the widget stuff is missing the |
|
Works! On Tue, Nov 5, 2013 at 4:40 PM, Jonathan Frederic
|
|
@cekees Could you do me a favor and pull the latest and test again when you get a chance (making sure to clear the browser cache)? I changed the JavaScript references yet again, but I am using |
|
I uploaded the basic widget tutorial notebooks, parts 1-5. They are listed in the PR description and actually stored in this PR (ipython/examples/widgets). |
|
Works for me, except Part 5 in the PR is giving me a 404 and on Part 4: Styles, In[5] I get the following error (there are also errors down further in the worksheet that may or may not be related. TypeError Traceback (most recent call last) TypeError: set_css() takes at least 3 arguments (2 given) |
Looks like you don't have the latest widgets.py? Commit 42c6fc8 changes |
Widget review
I think you are right. I merged your review PR, thanks for all the good input! Question: I noticed that, in the example notebooks, you returned after each period in the markdown cells. Is that something I should practice too? |
No, don't worry about it, it's just my habit. |
TextWidget, TextareaWidget, CheckboxWidget, and SelectWidget
|
@minrk and @ellisonbg this should be ready to merge after d661486 pleases our good friend Travis C. I. |
|
Merging. Great work @jdfreder !!! and thanks everyone for the review help, especially @minrk and @jasongrout @cekees |
IPEP 23: Backbone.js Widgets
|
Awesome! Congrats, guys! |
| // elsewhere. | ||
| define(["underscore", | ||
| "backbone", | ||
| ], function (Underscore, Backbone) { |
There was a problem hiding this comment.
This should be function(_, Backbone), since we are actually using underscore via _ below. This might also be a problem in other files.
|
Also, since we changed |
| closed = Bool(False) | ||
|
|
||
| keys = List() | ||
| def _keys_default(self): |
There was a problem hiding this comment.
So just to double-check this design decision---widget.keys is still the authoritative source for the syncing attributes, and setting the sync=True parameter just provides a convenient default? But we could just set widget.keys directly to change this behavior? (I had thought we were totally moving to the sync=True framework, but I do like the flexibility of manually overriding that architecture if I need to.)
There was a problem hiding this comment.
They keys attribute should always be constructed automatically by looking at the sync trait metadata. The problem with allowing the setting of keys manually is that it messes up subclassing.
There was a problem hiding this comment.
In that case, should keys be a property, rather than a traitlet?
There was a problem hiding this comment.
You can still set it manually, any classes that subclass that class won't be able to rely on sync = True.
There was a problem hiding this comment.
I think that breaking subclassing is always wrong, so we should make it a read only property? Thoughts?
There was a problem hiding this comment.
@minrk are you okay with using a readonly property for this?
There was a problem hiding this comment.
Sure, property is fine. Just make sure it's not evaluated on every request. I don't actually see any benefit to property, since there is really no such thing as read-only in Python.
There was a problem hiding this comment.
I don't actually see any benefit to property, since there is really no such thing as read-only in Python.
I agree. I'm indifferent to the property or traitlet approach.
There was a problem hiding this comment.
I think a traitlet implies that it makes sense to set it - a property suggests that you shouldn't try that, even though it's still technically possible.
There was a problem hiding this comment.
Can't you have a readonly traitlet? Basically, the validator just rejects any attempts to set it?
|
Thanks for pointing the naming issues out @jasongrout , I opened a PR to resolve them - #4933 |
|
Hi, Thanks a lot, PS: I am a first time poster so please pardon my lack of sophistication in my comments. I just started playing with GIT, IPython widgets, Backbone etc. |
|
It looks like the javascript unpacker should be checking to see if the value is an |
|
(perhaps using jquery isArray is safer than trying to deal with the javascript type system in lots of different browsers: http://api.jquery.com/jquery.isarray/) |
|
Thanks for pointing this out guys, I will be gone tomorrow. I'll try to open a PR to fix this over the weekend. |
|
@jdfreder: not to get impatient, but I'm curious if there is progress on this. I know you guys have been doing a ton of work lately, and we really appreciate it! |
|
Jason, can you open a new issue for us to track this one on? Thanks. |
|
Done: #5020 |
IPEP 23: Backbone.js Widgets
Description
Adds backbone.js layer on top of comm layer to enable the creation of modular widgets.
See IPEP 23 for a detailed description.
Organization
/ipython/IPython/html/static/js/widgetmanager.js/ipython/IPython/html/static/js/widgets/widget*.js/ipython/IPython/html/widgets/widget*.py/ipython/IPython/html/tests/casperjs/test_cases/widgets*.js/ipython/examples/widgets/*.ipynbExamples
Examples and tutorials are included in this PR's contents (as seen above) direct link
Included Widgets