feat(unstable): add `Deno.jupyter.broadcast` API #20656

bartlomieju · 2023-09-24T16:03:38Z

Closes #20591

bartlomieju · 2023-09-24T16:04:48Z

@rgbkrk this adds Deno.jupyter.sendIo API as we discussed in the issue, however sending anything (eg. await Deno.jupyter.sendIo({ "text/html": "<h1>Hello there!</h1>" });) has no effect, it's just null that's printed in the notebook. Am I doing something wrong, or did I misunderstand how this is supposed to work?

rgbkrk · 2023-09-24T16:45:45Z

It looks like you need more of the payload (data, metadata). I'll dive in shortly with this branch.

bartlomieju · 2023-09-24T18:03:35Z

Ooops, you're right! Even if I change the invocation to await Deno.jupyter.sendIo({ "data": {"text/plain": "<h1>Hello there!!!</h1>" } }); I still don't get anything printed :/ will continue to look what's wrong here.

EDIT: Ah, got it:

await Deno.jupyter.sendIo({ "data": {"text/html": "<h1>Hello there!!!</h1>" }, "metadata": { "text/htmll": {"foo": true } }});

Adding "metadata" does the trick and the message gets displayed:

rgbkrk · 2023-09-24T18:46:35Z

I think it's ok to show null as opposed to undefined. For me at least what would be helpful is getting back the created message to know that it made what I needed. The simpler layer on top gets built.

I still need some way to send update_display_data which has the same payload but the message type is different.

rgbkrk · 2023-09-24T18:49:21Z

runtime/js/90_deno_ns.js

+    async sendIo(content) {
+      await core.opAsync("op_jupyter_send_io", content);
+    },
+  },


🆒 now I know

rgbkrk · 2023-09-24T20:38:46Z

PR to add the ability to send an arbitrary IOPub message: bartlomieju#4

feat(jupyter): accept message type in jupyter io publish call

rgbkrk · 2023-09-24T20:43:01Z

This makes it nice and easy to build progress bars in the notebook and any other kind of report for users. Excited to see this coming in.

rgbkrk · 2023-09-25T04:42:07Z

I made sure other iopub messages worked:

`clear_output`

`stream`

`display_data` with `application/json`

`comm_open` and `comm_msg`

Of course, I can't tell how properly comm sending is working unless this is connected up with a receiver on the frontend.

`execute_result`

Generally I think this is great. I'm not a big fan of the name sendIo. It can look like lo in some fonts. I would at least capitalize the O as well in IO so it reads as sendIO.

Some alternatives I'm a fan of:

Deno.jupyter.iopub.send(...)

Deno.jupyter.broadcast(...)

Deno.jupyter.publishIO(...)

However, I don't wish to hold things up for the name of the function. Let's get this out there!

lucacasonato · 2023-09-25T06:53:09Z

runtime/js/90_deno_ns.js

@@ -178,6 +178,11 @@ const denoNsUnstable = {
  Kv: kv.Kv,
  KvU64: kv.KvU64,
  KvListIterator: kv.KvListIterator,
+  jupyter: {


I think Deno.jupyter should only exist if we're running within the context of deno jupyter

I actually want to make it a getter that will throw an informative error in non-Jupyter context 👍

rgbkrk · 2023-09-26T17:44:19Z

What else do we need for this? Tests?

I'm delighted to start using it within libraries.

bartlomieju · 2023-09-26T20:52:45Z

@rgbkrk no blockers, I'll try to finish it tonight so we can ship it in v1.37.1 tomorrow.

mmastrac · 2023-09-26T23:06:49Z

cli/js/40_jupyter.js

+function enableJupyter() {
+  denoNsUnstable.jupyter = {
+    async broadcast(msgType, content) {
+      await core.opAsync("op_jupyter_broadcast", msgType, content);


Nit: ensureFastOps here.

I think that makes no difference since it's using serde_json::Value which has no chance to ever be a fast call.

It's better performing than opAsync in general because it doesn't use the spread operator -- we generate overloads for the correct number of args.

In this case it's not critical but generally we should avoid using opAsync.

I'll change it in a follow up PR

mmastrac

LGTM

rgbkrk · 2023-09-27T00:13:08Z

cli/tests/unit/jupyter_test.ts

+    () => Deno.jupyter,
+    "Deno.jupyter is only available in `deno jupyter` subcommand.",
+  );
+});


How should we test the actual functionality under Deno.jupyter? Do we have to somehow monkeypatch to allow it then test it?

I added another cell in integration_test.ipynb notebook - if you run it manually there should be no changes in Git

Ahhhh beautiful

rgbkrk · 2023-09-27T00:13:20Z

cli/tsc/dts/lib.deno.unstable.d.ts

+     * @category Jupyter */
+    export function broadcast(
+      msgType: string,
+      content: Record<string, unknown>,


rgbkrk · 2023-09-27T04:38:36Z

cli/tests/testdata/jupyter/integration_test.ipynb

+     "traceback": [
+      "Stack trace:",
+      "TypeError: Cannot read properties of undefined (reading 'broadcast')",
+      "    at <anonymous>:2:20"


Oooops, git played a fool out of me. This should have been actual output. I will fix it in a follow up

bartlomieju added 2 commits September 24, 2023 17:46

wip

02316a6

feat(unstable): add sendio for Jupyter

657d730

don't print null

6ef320a

rgbkrk reviewed Sep 24, 2023

View reviewed changes

accept message type in send io

e072600

Merge pull request #4 from rgbkrk/jupyter-io-accept-message-type

0d71361

feat(jupyter): accept message type in jupyter io publish call

lucacasonato reviewed Sep 25, 2023

View reviewed changes

bartlomieju added 3 commits September 26, 2023 23:56

Merge branch 'main' into jupyter_stdio

f8ff4bc

finish the api, cleanup, add test

15b425d

Merge branch 'main' into jupyter_stdio

7b724e0

bartlomieju changed the title ~~[WIP] feat(unstable): add sendio for Jupyter~~ feat(unstable): add sendio for Jupyter Sep 26, 2023

reset CI

cad6fd8

bartlomieju changed the title ~~feat(unstable): add sendio for Jupyter~~ feat(unstable): add Deno.jupyter API Sep 26, 2023

bartlomieju changed the title ~~feat(unstable): add Deno.jupyter API~~ feat(unstable): add Deno.jupyter.broadcast API Sep 26, 2023

bartlomieju added 2 commits September 27, 2023 00:40

actually fix test

d7ff0de

lint

15adb0a

mmastrac reviewed Sep 26, 2023

View reviewed changes

mmastrac approved these changes Sep 26, 2023

View reviewed changes

rgbkrk reviewed Sep 27, 2023

View reviewed changes

bartlomieju merged commit 46a4bd5 into denoland:main Sep 27, 2023
13 checks passed

bartlomieju deleted the jupyter_stdio branch September 27, 2023 00:29

rgbkrk reviewed Sep 27, 2023

View reviewed changes

rgbkrk mentioned this pull request Oct 1, 2023

Deno Userland Jupyter Messaging library: comms #20592

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(unstable): add `Deno.jupyter.broadcast` API #20656

feat(unstable): add `Deno.jupyter.broadcast` API #20656

bartlomieju commented Sep 24, 2023

bartlomieju commented Sep 24, 2023

rgbkrk commented Sep 24, 2023

bartlomieju commented Sep 24, 2023 •

edited

rgbkrk commented Sep 24, 2023

rgbkrk Sep 24, 2023

rgbkrk commented Sep 24, 2023

rgbkrk commented Sep 24, 2023 •

edited

rgbkrk commented Sep 25, 2023 •

edited

lucacasonato Sep 25, 2023

bartlomieju Sep 25, 2023

rgbkrk commented Sep 26, 2023

bartlomieju commented Sep 26, 2023

mmastrac Sep 26, 2023

bartlomieju Sep 26, 2023

mmastrac Sep 26, 2023

bartlomieju Sep 27, 2023

mmastrac left a comment

rgbkrk Sep 27, 2023

bartlomieju Sep 27, 2023

rgbkrk Sep 27, 2023

rgbkrk Sep 27, 2023

rgbkrk Sep 27, 2023

bartlomieju Sep 27, 2023

feat(unstable): add Deno.jupyter.broadcast API #20656

feat(unstable): add Deno.jupyter.broadcast API #20656

Conversation

bartlomieju commented Sep 24, 2023

bartlomieju commented Sep 24, 2023

rgbkrk commented Sep 24, 2023

bartlomieju commented Sep 24, 2023 • edited

rgbkrk commented Sep 24, 2023

Choose a reason for hiding this comment

rgbkrk commented Sep 24, 2023

rgbkrk commented Sep 24, 2023 • edited

rgbkrk commented Sep 25, 2023 • edited

clear_output

stream

display_data with application/json

comm_open and comm_msg

execute_result

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgbkrk commented Sep 26, 2023

bartlomieju commented Sep 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmastrac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat(unstable): add `Deno.jupyter.broadcast` API #20656

feat(unstable): add `Deno.jupyter.broadcast` API #20656

bartlomieju commented Sep 24, 2023 •

edited

rgbkrk commented Sep 24, 2023 •

edited

rgbkrk commented Sep 25, 2023 •

edited

`clear_output`

`stream`

`display_data` with `application/json`

`comm_open` and `comm_msg`

`execute_result`