Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Consider Using Custom Jupyter Output Type #203

Closed
twavv opened this issue Sep 18, 2018 · 12 comments · Fixed by #211
Closed

Proposal: Consider Using Custom Jupyter Output Type #203

twavv opened this issue Sep 18, 2018 · 12 comments · Fixed by #211

Comments

@twavv
Copy link
Member

twavv commented Sep 18, 2018

Currently, things go very wrong (in my experience) if a notebook that is using WebIO is refreshed. Ipywidgets (for example) behaves better because it's rendering system is more Jupyter-aware.

Here's a simplified version of (my understanding of) what ipywidgets does.

  • You open a notebook with the ipywidgets nb-/labextension installed. The extension registers a renderer for the application/vnd.jupyter.widget-view+json MIME type.
  • Say you run import ipywidgets; s = ipywidgets.IntSlider() (this displays a slider that can take integer values).
  • The Python generates a display-data message with a application/vnd.jupyter.widget-view+json MIME type (which essentially just contains a reference to the widget's id) and sends a comm_open request to the browser associated with the id of that widget.
  • The renderer mentioned above is called, looks up a global widget_manager object, tries to find the comm associated with the widget (if it can't, it displays an error) and subscribes to updates associated with the widget id assigned to the slider.
  • You refresh the page.
  • The extension initialization code does a check with the kernel (it sends a comm_info_request, which lists all the open comms) to see if any widget comms are registered.
  • The notebook is initialized further, and the widgets are rendered via the same process described above.

We could (and I think should) do something similar (i.e. wrapping WebIO content in a application/vnd.juliagizmos.webio.node+json, or something similar, display MIME type; this would require a slight modification to IJulia itself, to allow displaying arbitrary MIME types, but nothing major).

This also has the advantage of not having to do all the WebIO.mount stuff inside the display data message, which seems kind of hacky. It would also not require the use of a custom asset registry (which I also think seems very hacky in the context of Jupyter; no other notebook functionality that I'm aware of hooks up its own asset server).

I'd be happy to help (I do consider myself to be intimately familiar with the Jupyter protocol and a pretty decent web-oriented developer, much moreso that with Julia). I'm also happy to answer questions about why I think this is a good idea if you have any.

I also confess that I have an ulterior motive in that I'm trying to develop a custom Jupyter web-based frontend (for an education-related project) and really want to be able to use Julia/Interact/WebIO, but find the current state of things untenable.

tag @shashi

@twavv
Copy link
Member Author

twavv commented Sep 18, 2018

This could also be implemented in a way that solves #139.

@shashi
Copy link
Member

shashi commented Sep 18, 2018

Reload works in JupyterLab, but #139 is a problem there.

I think we can fix reload in Jupyter too. We don't necessarily have to implement a mime type extension for that, just a jupyter js extension. I have started working on one in my fork of IJulia. https://github.com/JuliaLang/IJulia.jl/compare/master...shashi:s/webio?expand=1 You're welcome to dive in if you have the time. Otherwise I will get to it in due time.

@shashi
Copy link
Member

shashi commented Sep 18, 2018

#139 is a separate issue, it happens because we put state (such as communication callback) in a global WebIO object. It should be higher priority at the moment.

@twavv
Copy link
Member Author

twavv commented Sep 18, 2018

I suppose it's somewhat a matter of philosophy but I really don't think that the asset server is the way to go in Jupyter given that the notebook/JupyterLab has dedicated ways of delivering Javascript to the frontend.

And I advocate for the MIME type based rendering both because I think it's a bit simpler/more elegant and because it's the more Jupyter-y way of doing things (again, this is what every other library that I know of does). This is definitely that path of least resistence in terms of being able to add support for other Jupyter-based frontends (there exist more than just the notebook, such as nteract), and something I'm trying to do myself.

Also, I'm not sure how you'd fix #139 without something like what I'm talking about, especially considering that JupyterLab doesn't provide very much (any?) access to its internals from outside of dedicated extension entrypoints. If you execute Javascript from within the context of the cell (via a <script />), how does the script get access to the relevant WebIO object?

@shashi
Copy link
Member

shashi commented Sep 18, 2018

I suppose it's somewhat a matter of philosophy but I really don't think that the asset server is the way to go in Jupyter given that the notebook/JupyterLab has dedicated ways of delivering Javascript to the frontend.

Sure, but two things:

  1. We need a system that works not only in Jupyter but other front ends too.
  2. Jupyter plugins use npm to install stuff. Supporting something like this will require WebIO to also install npm outside of Jupyter's own installation of it for the sake of other front ends. The (fair) assumption is that the user doesn't care how widgets work as long as they work.

WebIO currently uses SystemJS for loading JS modules - this has been relatively problem-free for some time now.

And I advocate for the MIME type based rendering both because I think it's a bit simpler/more elegant and because it's the more Jupyter-y way of doing things (again, this is what every other library that I know of does). This is definitely that path of least resistence in terms of being able to add support for other Jupyter-based frontends (there exist more than just the notebook, such as nteract), and something I'm trying to do myself.

Currently we don't just have JSON as the output of WebIO. We also have expressions such as

{
  tag: "button",
  events: {
    "click": function () { alert("hi"); }
   }
}

That's why we print <script> tags.

Also, I'm not sure how you'd fix #139 without something like what I'm talking about, especially considering that JupyterLab doesn't provide very much (any?) access to its internals from outside of dedicated extension entrypoints. If you execute Javascript from within the context of the cell (via a <script />), how does the script get access to the relevant WebIO object?

We already have a JupyterLab JavaScript plugin https://github.com/JuliaGizmos/WebIO.jl/blob/master/assets/webio/jupyterlab_entry.js

The way to provide a WebIO instance per kernel is to tack it on to the kernel object there / in a dictionary keyed by kernel id. Widgets use their respective kernel to talk to Julia.

Overall, I agree with you that Jupyter(Lab) support is not perfect, but it can be fixed in these two steps.

  1. Fix Cross-talk, warnings, and errors with multiple jupyterlab tabs #139
  2. Make a Jupyter JS plugin

I will assume that one of the plugins (either JupyterLab or Jupyter) will get things to work on nteract as well. (I will not be surprised if this already works, in fact I would be mildly surprised if it doesn't)

However, any new mime type would have to be identical to text/html.

@twavv
Copy link
Member Author

twavv commented Sep 18, 2018

We need a system that works not only in Jupyter but other front ends too.

Absolutely! What I'm proposing is just, in the case of IJulia, defining a custom MIME type that the Jupyter frontend can render (we could even add a new MIME type to the bundle that IJulia delivers, and leave the current text/html as it is, and let the frontend choose from text/html and text/vnd.whatever.webio+html). I think this fits a lot better into the motivation behind Jupyter and wouldn't negatively impact any other frontends.

The way to provide a WebIO instance per kernel is to tack it on to the kernel object there / in a dictionary keyed by kernel id. Widgets use their respective kernel to talk to Julia.

I'm not aware of any way for code within an output to determine which kernel it's attached too. So we could define a kernel id => WebIO instance map, but then how does the javascript code know which kernel it's attached to to figure out which WebIO instance to use? Also, registering window globals is considered very bad practice in modern code.

@shashi
Copy link
Member

shashi commented Sep 18, 2018

The kernel id will have to be printed by Julia. It's easy to send the kernel id to Julia when establishing a connection.

We may have to use something like a weak key dictionary, but not too important at the moment.

@twavv
Copy link
Member Author

twavv commented Sep 18, 2018

I suppose you could do that, but isn't that kind of like trying to fit a square peg into a round hole at this point? Are there any reasons why you think that would be better than having a MIME type that Jupyter can render using a notebook/lab extension?

And that wouldn't that (the weak key dictionary) require modifying the code for all the providers to have knowledge of this dictionary? Don't most of the other providers just care about a single, global WebIO instance?

What I think would be the optimal solution would be to define a MIME type (call it WEBIO_MIME) and have a show function like

function Base.show(io::IO, m::MIME"application/vnd.webio.v1+json", x::Node)
    write(io, json(
        "node" => escapeHTML(sprint(s->jsexpr(s, x))),
    ))
end

(probably leaving the normal show function for text/html completely alone, so that the other providers are completely unaffected)
and then a Jupyter renderer (via the plugin) that can then be something like (I'm not super familiar with the plugin interface for JupyterLab) this.

const WEBIO_MIME_TYPE = "application/vnd.webio.v1+json";
class WebIOPlugin {
  createNew(nb: NotebookPanel) {
    this.nb = nb;
    this.comm = nb.create_comm(...);
    this.WebIO = new WebIO(...); // set up using comm
    nb.rendermime.addFactory({
      safe: false,
      mimeTypes: [WEBIO_MIME_TYPE],
      createRenderer: (options) => new WebIORenderer(options, this.WebIO),
  });
}
class WebIORenderer extends Widget {
  constructor(options, WebIO) {
    super(options);
    this.WebIO = WebIO;
  }
  async renderModel(model: IRenderMime.IMimeModel) {
    // model.data is the JSON blob that we send
    const data = model.data[WEBIO_MIME_TYPE];
    // this.node is created by JupyterLab
    this.WebIO.mount(this.node, this.data["node"]);
  }
}

The above was modeled (somewhat loosely) on the plugin class and the renderer class from ipywidgets.

No global variables, nor any maps, required.

@twavv
Copy link
Member Author

twavv commented Sep 20, 2018

Hmmm, I'm not sure how this would relate to the current asset registry. This is tangentially related (and I'm aware it's been discussed before in another issue, in #124).

To be clear, I don't think that a custom asset registry should be in use with Jupyter, especially since the kernel is not guaranteed to be located on the same machine as the notebook (eg. when using Jupyter enterprise gateway, and likely in the future when collaborative notebooks are introduced). This is a sticking point for me since we run kernel processes in separate docker containers distributed across a Kubernetes cluster.

It's also a sticking point in nteract and other frontends (such as QtConsole, though I'm not sure that WebIO would ever work in QtConsole considering that it's not web-based) since nteract only launches the kernel processes (no Jupyter notebook processes).

Re #124, I think it's would be possible to have SystemJS load modules from strings by writing a simple SystemJS plugin.

@shashi
Copy link
Member

shashi commented Sep 21, 2018

Re #124, I think it's would be possible to have SystemJS load modules from strings by writing a simple SystemJS plugin.

That's interesting, it would be interesting to explore that. I think it would mean we may have to give up loading assets asynchronously which may or may not turn out to be a big problem.

especially since the kernel is not guaranteed to be located on the same machine as the notebook (eg. when using Jupyter enterprise gateway, and likely in the future when collaborative notebooks are introduced).

interesting situation... My first reflex would be to use Mux to serve assets in this case.

What I think would be the optimal solution would be to define a MIME type (call it WEBIO_MIME) and have a show function like

This definitely sounds OK. Keep in mind however that the JSON is not purely JSON, it has JS interspersed in it...

E.g.

{
  tag: "button",
  events: {
    "click": function () { alert("hi"); }
   }
}

@twavv
Copy link
Member Author

twavv commented Sep 22, 2018

My first reflex would be to use Mux to serve assets in this case.

I might be missing something but I'm not sure how this would help, since Mux would be running on the same host as the kernel (so you couldn't just run Mux on port 12345 and point the browser to localhost:12345/assets/... ). I'm also not sure what you mean by loading scripts asynchronously.

This definitely sounds OK. Keep in mind however that the JSON is not purely JSON, it has JS interspersed in it...

Can we just represent the function as a string?

{
  tag: "button",
  events: {
    "click": "function () { alert(\"hi\"); }"
   }
}

Another issue, that needs to be solved one way or another, is that WebIO really shouldn't be a window global (this is really the root of the JupyterLab issue).

I'm not really sure what the state of JSExpr is and the docs don't make it clear (for example, the @js stuff on the README doesn't work without JSExpr). It seems that JSExpr depends on WebIO, and it emits code the depends on a WebIO global. To make it work with multiple WebIO's on the same browser window, you'd have to do some fancy scope mapping (maintain a map of scope id's to WebIO instances).
@js $obs[] = Math.random() emits code along the lines of

JSString("WebIO.setval({\"name\":\"foo\",\"scope\":\"scope-85656c18-5fbf-4f63-b741-3c1a137b2281\",\"id\":\"ob_01\",\"type\":\"observable\"},Math.random())")

So, one way to get around this would be to evaluate the code in a context where the WebIO is a local variable and rely on Javascript closures, but I'm not sure that's a great idea either. This would be something along the lines of...

const callbackSource = `function (event) { WebIO.setval(...); }`
const WebIO = // get the correct WebIO;

// Wrap the function declaration in parens so JS treats it as an expression and returns the Function instance
// When eval'd, WebIO is captured from the current scope
// Bind to the DOM node so that `this` is always correct
const callback = eval(`(${callbackSource})`).bind(someDOMNode);

// When executed, WebIO is accessed via the closure created above, no window globals required
someDOMNode.addEventListener("click", callback);

@twavv
Copy link
Member Author

twavv commented Sep 28, 2018

@shashi Is there any particular reason that whenever an observable updates, the whole <script>...</script> bundle is sent over the comm rather than just the new value of the observable? It also seems like the latter (only sending the value) would get rid of a lot of escaping issues.

i.e. instead only send something like

{
   type: "message",
  command: "update-observable", 
  scope: "scope-id",
  data: "representation-of-new-value",
}

over the comm and let the already set-up observable handle changing the displayed value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants