Skip to content
This repository has been archived by the owner on Oct 26, 2019. It is now read-only.

[Issue 23] Support Declarative Widgets #62

Merged
merged 12 commits into from
Feb 15, 2016

Conversation

jhpedemonte
Copy link
Collaborator

Ref #23

Based on jupyter/declarativewidgets#157, which attempts to get Declarative Widgets running on new APIs. Had to make the following changes to that code:

  1. In elements/urth-core-bind/dom-bind-behavior.html, change Polymer.DomApi to Polymer.TreeApi. This may have already been fixed in DeclWidgets master.
  2. In nb-extension/js/widgets/DeclWidgetModel.js, replace use of nbextensions/widgets/widgets/js/manager & nbextensions/widgets/widgets/js/widget with jupyter-js-widgets

To build:

  1. make build -- 2nd container will fail
  2. make run-kernel-gateway-with-widgets
  3. make dev
  4. Load http://localhost:3000/notebooks/decl-widgets

Status: Shows decl widgets but they don't function (error in console when clicking Invoke button).

@jhpedemonte
Copy link
Collaborator Author

Changes to Dockerfile.kernel and Makefile are definitely hacky and should be replaced. All necessary shims are currently in our WidgetManager.

@jhpedemonte jhpedemonte changed the title WIP: Support Declarative Widgets [WIP] [Issue 23] Support Declarative Widgets Feb 4, 2016
@jhpedemonte
Copy link
Collaborator Author

After latest changes and rebase to master, I'm now seeing this error on KG server (even with non-declwidgets notebook, like simple.ipynb):

[IPKernelApp] ERROR | Exception opening comm with target: jupyter.widget
Traceback (most recent call last):
  File "/opt/conda/lib/python3.4/site-packages/ipykernel/comm/manager.py", line 116, in comm_open
    f(comm, msg)
  File "/opt/conda/lib/python3.4/site-packages/ipywidgets/widgets/widget.py", line 140, in handle_comm_opened
    class_name = str(msg['content']['data']['widget_class'])
KeyError: 'widget_class'

@jhpedemonte
Copy link
Collaborator Author

Error is that python code is looking for content.data.widget_class in comm_open WS message, but client is sending it as metadata.widget_class.

@jhpedemonte
Copy link
Collaborator Author

Heh, so looks like other bits of code expect the metadata to be passed in as the data when creating a new comm.

From jupyter-js-widgets, see:

@parente
Copy link
Member

parente commented Feb 9, 2016

Heh, so looks like other bits of code expect the metadata to be passed in as the data when creating a new comm.

Need to open this against ipywidgets.

/cc @jdfreder

@jhpedemonte
Copy link
Collaborator Author

Have simple example at /data/decl-widgets.ipynb now working, when building against my branch of declwidgets.

The full Walkthrough.ipynb from declwidgets does not work. So still some work to be done.

I tried to clean things up by automating cloning/building/installing of declwidgets (like we do for ipywidgets now), but couldn't get it going. For one, declwidgets currently depends on a local sibling copy of ipywidgets in package.json.

cellData.get_callbacks = function() {
return that._get_callbacks(outputAreaModel);
};
$cell.data('cell', cellData);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not 100% sure if we need this block of code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a mimic of what the notebook provides today. Not sure decl widgets can do anything differently at the moment if it is to remain compatible with existing notebook.

@jhpedemonte
Copy link
Collaborator Author

With latest code:

  1. Fully contained dependency on declwidgets (no sibling git repo required).
  2. make dev and make demo-container both work.
  3. test and integration-test both pass.
  4. Travis is green.

@jhpedemonte jhpedemonte changed the title [WIP] [Issue 23] Support Declarative Widgets [Issue 23] Support Declarative Widgets Feb 11, 2016
@jhpedemonte
Copy link
Collaborator Author

Added a couple of declwidgets examples. To use the taxi demo, you'll need to run:

make examples

This is because this demo requires some additional Polymer elements which aren't part of the base declwidgets installation.

@@ -110,8 +130,10 @@ define(['jupyter-js-widgets'], function(Widgets) {
* metadata: ???
*/
WidgetManager.prototype._create_comm = function(targetName, id, metadata) {
// TODO Other Jupyter code seems to expect `metadata` to be passed in as `data`. Why?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the args are misaligned, can you open this upstream in ipywidgets?

@parente
Copy link
Member

parente commented Feb 12, 2016

After hacking around the .PHONY problem for make examples, I brought everything up with make dev and got an exception before even visiting a dashboard:

ERROR in Entry module not found: Error: Cannot resolve 'file' or 'directory' ./node_modules/urth-widgets/index.js in /Users/parente/projects/jupyter/dashboards_nodejs_app
[12:51:54] 'webpack:components' errored after 1.59 s
[12:51:54] Error: during webpack compilation
    at /Users/parente/projects/jupyter/dashboards_nodejs_app/gulpfile.js:69:22
    at Compiler.<anonymous> (/Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/webpack/lib/Compiler.js:194:14)
    at Compiler.emitRecords (/Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/webpack/lib/Compiler.js:282:37)
    at Compiler.<anonymous> (/Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/webpack/lib/Compiler.js:187:11)
    at /Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/webpack/lib/Compiler.js:275:11
    at Compiler.applyPluginsAsync (/Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/tapable/lib/Tapable.js:60:69)
    at Compiler.afterEmit (/Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/webpack/lib/Compiler.js:272:8)
    at Compiler.<anonymous> (/Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/webpack/lib/Compiler.js:267:14)
    at /Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/webpack/node_modules/async/lib/async.js:52:16
    at done (/Users/parente/projects/jupyter/dashboards_nodejs_app/node_modules/webpack/node_modules/async/lib/async.js:246:17)

@parente
Copy link
Member

parente commented Feb 12, 2016

After a:

make clean
make examples

I got a message saying nothing to be done. Which is odd. Turns out the taxi demo is still in data (should be added to make clean). So I manually removed it, then tried make examples again. This time I got:

/bin/sh: line 0: cd: public/urth_components: No such file or directory
make: *** [data/decl-widgets-taxi-demo.ipynb] Error 1

Sure enough, there's no urth_components in there. What should be populating that?

@jhpedemonte
Copy link
Collaborator Author

Updated patch with minor Makefile fixes to address @parente's build issues.

@parente
Copy link
Member

parente commented Feb 15, 2016

make clean
make dev-install
make examples
make kill # so no old kernel gateway image is running!
make dev

The server starts and most things work. I hit a few problems running the examples. Not sure if these are integration problems or decl widgets problems (cc @lbustelo). I did see that the Dockerfile.kernel is doing a pip install of the latest stable declarative widgets, but the Dockerfile.server is git cloning your branch. Mismatch between frontend and backend or is the backend (Python kernel-side) untouched?

decl-widgets.ipynb

Works.

decl-widgets-chart.ipynb

Example 9 does not function. Get an error in the browser JS console when I pick from the drop downs.

TypeError: Cannot read property 'map' of undefined(…)

decl-widgets-taxi-demo.ipynb

After i pick a trip and click Load, the map / table did not show up in the next big blank area the first time I tried. I got this error in the console:

Uncaught Error: Function arguments are not in valid state

I refreshed and tried again, this time letting the page settle for a while before clicking. This time it worked. I think the first time, I picked something from the dropdown ahead of everything being fully initialized in the front and backend. The good old race condition of async frontend loads vs backend execution.

The plot and other widgets further down the dashboard page do show up and work fine.

test_layout_declarativewidgets.ipynb

The widgets appear. The dropdown widget shows choices, but doesn't allow me to select anything. No errors logged anywhere.

@jhpedemonte
Copy link
Collaborator Author

Example 9 does not function.

This is also an error when running on jupyter.cloudet.xyz. I think @lbustelo said it was a "run all" issue, which should now be fixed on declwidgets master.

decl-widgets-taxi-demo.ipynb

I think this may also be a "run all" problem.

test_layout_declarativewidgets.ipynb

Don't remember this one. Will check.

@jhpedemonte
Copy link
Collaborator Author

test_layout_declarativewidgets.ipynb

Uploaded to jupyter.cloudet.xyz and the dropdown menu functions the same in the notebook as it does on our server: clicking items in dropdown menu does nothing.

Ref jupyter#23

(c) Copyright IBM Corp. 2016
(c) Copyright IBM Corp. 2016
Also, don't build jquery into components.

(c) Copyright IBM Corp. 2016
(c) Copyright IBM Corp. 2016
... instead of depending on local sibling dir

(c) Copyright IBM Corp. 2016
On KG, we only need Python side of things, so no need to install
declwidgets from source branch.

(c) Copyright IBM Corp. 2016
... rather than depending on sibling git repo

(c) Copyright IBM Corp. 2016
(c) Copyright IBM Corp. 2016
(c) Copyright IBM Corp. 2016
To make use of taxi demo, need to run `make examples`

(c) Copyright IBM Corp. 2016
Move 'examples' folder to 'etc/notebooks'

(c) Copyright IBM Corp. 2016
@parente
Copy link
Member

parente commented Feb 15, 2016

OK. I'm good with merging this as-is then. It's in the spirit of the original issue: simple demo to show the widgets working. You've gone above and beyond. 👍

parente added a commit that referenced this pull request Feb 15, 2016
[Issue 23] Support Declarative Widgets
@parente parente merged commit f3290c3 into jupyter:master Feb 15, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants