Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

[WIP] Standalone widgets #403

Closed
wants to merge 4 commits into from
Closed

[WIP] Standalone widgets #403

wants to merge 4 commits into from

Conversation

Lull3rSkat3r
Copy link
Member

@Lull3rSkat3r Lull3rSkat3r commented Jun 16, 2016

This PR is meant to allow declarative widgets to be installed as an npm module and be used in standalone applications.

So to fully integrate a config like this will need to be passed in as such:

// `_kernel` is going to be an object created from http://jupyter.org/jupyter-js-services/globals.html#startnewkernel 

// See feedback #2
_kernel.is_connected =  function(){ return true; };

// See feedback #3
var notebook = {
      base_url : '/bower_components',
      kernel : _kernel
};

// See feedback #4
_kernel.widget_manager = new WidgetManager(notebook, function(msg){});

    // DeclWidgets integration
    this._declWidgets = DeclWidgets.init({
      namespace : {
          notebook : notebook
      },
      componentsDir : '' // See feedback #5
});

Feedback Points

  1. postinstall.js - This file will run when after npm install. It requires bower to be installed and will run bower install for all of the bower components defined in the elements directory. This is the best solution I could find at the time, but am open to discussion for better solutions.
  2. The notebook kernel object seems to differ from that of jupyter-js-services in terms of defining when a kernel is connected. I would like to align this internally to declarative widgets to be around the jupyter-js-services API. If this seems like the appropriate way to go will add that to this PR.
  3. We need to create a notebook object, which may seem confusing outside of the notebook work. Don't know if we want to support to different options: one for the notebook; one for standalone applications. Note: This may be a breaking change if we do not support both methods. For backwards compatability we will need to keep the notebook method.
  4. Each app will need to make a widget manager. It would be nice if there was a default one that did nothing
  5. init.js - I needed a way to be able to configure the COMPONENTS_DIR variable for outside applications. I solved this through a config value here

Edit: Updated bullet point 3 to reflect this may be a breaking change

@Lull3rSkat3r
Copy link
Member Author

@parente @deedubbu @lbustelo, looking for feedback on the points above.

@parente
Copy link
Member

parente commented Jun 17, 2016

The notebook kernel object seems to differ from that of jupyter-js-services in terms of defining when a kernel is connected. I would like to align this internally to declarative widgets to be around the jupyter-js-services API. If this seems like the appropriate way to go will add that to this PR.

Pretty sure this is what we did in dashboards server, then shimmed it for libs that wanted the old kernel UI.

We need to create a notebook object, which may seem confusing outside of the notebook work. Don't know if we want to support to different options: one for the notebook; one for standalone applications. Note: This may be a breaking change if we do not support both methods. For backwards compatability we will need to keep the notebook method.

If decl widgets starts creating a notebook object, it's going to also be aware that dashboards server creates a notebook object and not to mess with it.

Each app will need to make a widget manager. It would be nice if there was a default one that did nothing

Sounds like the shims both dashboard and notebook server puts in place for ipywidgets are now going into decl widgets? That seems wrong.

Overall, I'm a bit worried that we're replicating all the gorp we had to put into dashboards server in decl widgets so that they carry a notebook-like environment with them wherever they go. If the goal is to allow any Python, R, Scala, etc. code to be used with any lib in the ecosystem, the shimming will continue. Do we want all of this in decl widgets? Can we scope what's supported vs what's not?

window.Polymer.dom = 'shadow';
// NOTE: This breaks some code because it is expecting Polymer.dom to be a function
// window.Polymer.dom = 'shadow';
window.Polymer.dom = window.Polymer.dom ? window.Polymer.dom : 'shadow';
Copy link
Collaborator

Choose a reason for hiding this comment

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

@deedubbu There many be a different way of setting shadow dom since now Polymer moved the dom api to the top of the Polymer global.

@Lull3rSkat3r
Copy link
Member Author

Overall, I'm a bit worried that we're replicating all the gorp we had to put into dashboards server in decl widgets so that they carry a notebook-like environment with them wherever they go.
@parente agreed. I think the shims should be removed and unified. I can scope out the shims in this item and discuss what makes sense to add to this PR.

@lbustelo
Copy link
Collaborator

I agree with @parente. We do not want the environment shimming here. I'm ok with evaluating how we are injecting the dependencies on init and open to suggestions.

@Lull3rSkat3r
Copy link
Member Author

Spoke with @lbustelo and we decided to align on the API:

var kernel = jupyter-js-services.startKernel();

// Config values needed for declarativewidgets
var config = {
      base_url : '/bower_components',
      components_url : '',
};

var declWidgets = DeclWidgets.init(kernel, config);

This will require creating shim for just the notebook environment. This will be contained in the declwidgets notebook extension.

As @parente mentioned, once we finalize this API I will create a PR in dashboards server to align with this new API.

We are still missing the WidgetManager piece, but we will add that after this next pass is approved.

@parente
Copy link
Member

parente commented Jun 21, 2016

As @parente mentioned, once we finalize this API I will create a PR in dashboards server to align with this new API.

Is there any way to keep backwards compat in the API? Can kernel just be passed as a property of config?

@Lull3rSkat3r
Copy link
Member Author

That could be done, but that will require shimming to be done inside declarativewidgets proper and not the nbextension (i.e. in init.js and not main.js). IMO it will be easier to maintain a shim in main.js than init.js and will remove shimming across projects that consume widgets.

Fixed minor bug with Polymer.dom not being a function.
(c) Copyright IBM Corp. 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.

None yet

3 participants