vidartf / phoila

BSD 3-Clause "New" or "Revised" License
9 stars 5 forks source link

Supporting renderers that use comms #7

Open saulshanabrook opened 5 years ago

saulshanabrook commented 5 years ago

I have a lab extension that adds a renderer that uses comms. It displays interactive vega charts that call back to the backend to recompile expressions as the user interacts with the data (https://github.com/Quansight/ibis-vega-transform).

Currently it uses the notebook tracker to get the kernel for the current kernel and register a comm handler

https://github.com/Quansight/ibis-vega-transform/blob/c59d36ab575e0f0eb5d6fbee5e2aa8b420cde6d9/src/index.ts#L28-L40

This was inspired by @gnestor's PR to add even handling support to the vdom extension which was in turn inspired by @jasongrout

What would be the best way to support this in phoila? One option would be to add the notebook tracker extension and keep the extensions the same. Another option would be for JupyterLab to expose a higher level way of creating a comm in a mime render extension.

vidartf commented 5 years ago

This is a good question! Currently, ipywidgets gets special cased, but we should ideally find a solution that works for all Comm-based mime-renderers with all contexts that have a kernel associated with it.

While this is being solved, a workaround is to require the TVoilaTracker tracker, and use this to get the kernel:

tracker: TVoilaTracker;

tracker.widgetAdded.connect((tracker_, widget) => {
  (await widget.content.session.connected).kernel
});
saulshanabrook commented 5 years ago

Thank you vidar for the workaround, that's great! So I guess for an extension that supports both lab and phoila it could require the voila tracker notebook tracker and make them both optional

vidartf commented 5 years ago

So I guess for an extension that supports both lab and phoila it could require the voila tracker notebook tracker and make them both optional

Yup, but I still think we should try to come up with a more generic solution to this. It could help clean up some of the logic in the ipywidgets lab manager as well! cc @jasongrout

saulshanabrook commented 5 years ago

@vidartf unfortunately I don't see the content attribute on the widget, either in TS or at runtime:

    voila.widgetAdded.connect((_, widget) => {
      console.log(widget);
    });
Screen Shot 2019-10-08 at 12 14 13 PM
vidartf commented 5 years ago

Ah, it might not have made it to a released version yet. A new version is right around the corner though.

saulshanabrook commented 5 years ago

Thank you Vidar. Let me know if I can help with anything.