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

feat(unstable): add Deno.jupyter.display API #20819

Merged
merged 20 commits into from Oct 12, 2023

Conversation

rgbkrk
Copy link
Contributor

@rgbkrk rgbkrk commented Oct 7, 2023

This brings in display as part of the Deno.jupyter namespace. I went ahead and hooked up our execute_result handling to use Deno.jupyter.format so that DataFrames, Canvas, and other objects can be displayed nicely (even without implementation in the respective libraries).

image

cli/tools/jupyter/server.rs Outdated Show resolved Hide resolved
@bartlomieju
Copy link
Member

bartlomieju commented Oct 8, 2023

How can I keep the type information around? Also, I'd love to be able to have documentation of these functions, which is not clear to me on how we expose it for these conditionally hidden functions (since they're only available when running the deno jupyter kernel).

Could you elaborate what do you mean by type information here? In general when new APIs are added then the type declarations shipped with Deno should be updated - the Deno.jupyter namespace is defined in cli/tsc/dts/lib.deno.unstable.d.ts around line 2080.

Could we migrate to emitting the execute result inside of the evaluation result handling directly? Then we wouldn't have to do the extra serializing and deserializing of the media bundles.

Yes, I think we can do that with Deno.jupyter.broadcast API.

How can I includes tests for display, format, etc? I wrote tests for them in https://github.com/rgbkrk/display.js as deno package development itself makes it really simple.

We can add Deno.jupyter.format to internals symbol and then use it in a test file. This is already done in 40_jupyter.js:

internals.enableJupyter = enableJupyter;

Then in a test file you can do: const jupyterFormat = Deno[Deno.internal].jupyterFormat to get access to this function.

@rgbkrk
Copy link
Contributor Author

rgbkrk commented Oct 8, 2023

In general when new APIs are added then the type declarations shipped with Deno should be updated - the Deno.jupyter namespace is defined in cli/tsc/dts/lib.deno.unstable.d.ts around line 2080.

I can update the type definitions. Are we not able to write typescript that would get transpiled and then included in the resulting Deno binary?

Could we migrate to emitting the execute result inside of the evaluation result handling directly? Then we wouldn't have to do the extra serializing and deserializing of the media bundles.

Yes, I think we can do that with Deno.jupyter.broadcast API.

What's the best way for me to pass the execution count into the Runtime.callFunctionOn call?

Then in a test file you can do: const jupyterFormat = Deno[Deno.internal].jupyterFormat to get access to this function.

Awesome, I'll do that.

@rgbkrk rgbkrk force-pushed the bring-display.js-in branch 4 times, most recently from f297f24 to be1771b Compare October 8, 2023 22:26
@bartlomieju
Copy link
Member

I can update the type definitions. Are we not able to write typescript that would get transpiled and then included in the resulting Deno binary?

You can write TypeScript in internal code and it will get transpiled automatically, but for public APIs (think all Deno.* APIs) we need to write these type defs manually anyway.

What's the best way for me to pass the execution count into the Runtime.callFunctionOn call?

Create a CallArgument like so:

CallArgument {
  value: Some(Value::Number(execution_count)),
  ..Default::default()
}

and pass it in arguments vector to Runtime.callFunctionOn.

@rgbkrk rgbkrk force-pushed the bring-display.js-in branch 3 times, most recently from 796bddb to 169dffc Compare October 10, 2023 19:50
@rgbkrk rgbkrk marked this pull request as ready for review October 10, 2023 22:29
@rgbkrk
Copy link
Contributor Author

rgbkrk commented Oct 10, 2023

cc @manzt -- bringing display.js into Deno.jupyter. I'd love your perspectives as we seek to include display as a built in to Deno.

@manzt
Copy link
Contributor

manzt commented Oct 11, 2023

Super exciting! Thanks for pinging me. Maybe there is a way anywidget could also integrate more deeply but for now I think it makes sense to keep the APIs in userland.

@rgbkrk
Copy link
Contributor Author

rgbkrk commented Oct 11, 2023

Maybe there is a way anywidget could also integrate more deeply but for now I think it makes sense to keep the APIs in userland.

The big piece we likely need for anywidget is going to be the comm sending and receiving piece. Given deno's commitment to web standards it seems like we'd build on EventTarget and likely expose both a CommManager (Registry?) as well as a simple Comm class. Some portion of it probably belongs in userland for speed of iteration and minimalism in the kernel, however I think the Comm stuff is a bit inescapable for needing kernel side implementation.

cli/tsc/dts/lib.deno.unstable.d.ts Show resolved Hide resolved
cli/tests/testdata/jupyter/display_test.ts Outdated Show resolved Hide resolved
cli/tests/testdata/jupyter/format_test.ts Outdated Show resolved Hide resolved
cli/tests/testdata/jupyter/format_test.ts Outdated Show resolved Hide resolved
@bartlomieju bartlomieju changed the title feat(jupyter): expose display in deno proper feat(unstable): add Deno.jupyter.display API Oct 12, 2023
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM, this is a fantastic functionality @rgbkrk 🚀

@bartlomieju bartlomieju enabled auto-merge (squash) October 12, 2023 22:15
@bartlomieju bartlomieju merged commit 48e695a into denoland:main Oct 12, 2023
13 checks passed
bartlomieju added a commit that referenced this pull request Oct 12, 2023
This brings in [`display`](https://github.com/rgbkrk/display.js) as part
of the `Deno.jupyter` namespace. 

Additionally these APIs were added:
- "Deno.jupyter.md"
- "Deno.jupyter.html"
- "Deno.jupyter.svg"
- "Deno.jupyter.format"

These APIs greatly extend capabilities of rendering output in Jupyter
notebooks.

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
@rgbkrk rgbkrk deleted the bring-display.js-in branch October 12, 2023 23:43
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 this pull request may close these issues.

None yet

4 participants