-
Notifications
You must be signed in to change notification settings - Fork 329
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
improvement: patch IPython.display.display
to call mo.output.append
#919
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
# Copyright 2024 Marimo. All rights reserved. | ||
from __future__ import annotations | ||
|
||
import functools | ||
from typing import Any, Callable | ||
|
||
from marimo._messaging.mimetypes import KnownMimeType | ||
from marimo._output import builder | ||
from marimo._output.formatters.formatter_factory import FormatterFactory | ||
|
||
|
||
class IPythonFormatter(FormatterFactory): | ||
@staticmethod | ||
def package_name() -> str: | ||
return "IPython" | ||
|
||
def register(self) -> Callable[[], None]: | ||
import IPython.display # type:ignore | ||
|
||
from marimo._output import formatting | ||
from marimo._runtime.output import _output | ||
|
||
old_display = IPython.display.display | ||
# monkey patch IPython.display.display, which imperatively writes | ||
# outputs to the frontend | ||
|
||
@functools.wraps(old_display) | ||
def display(*objs: Any, **kwargs: Any) -> None: | ||
# IPython.display.display returns a DisplayHandle, which | ||
# can be used to update the displayed object. We don't support | ||
# that yet ... | ||
if kwargs.pop("clear", False): | ||
_output.clear() | ||
for value in objs: | ||
_output.append(value) | ||
|
||
IPython.display.display = display | ||
|
||
def unpatch() -> None: | ||
IPython.display.display = old_display | ||
|
||
@formatting.formatter(IPython.display.HTML) | ||
def _format_html( | ||
html: IPython.display.HTML, | ||
) -> tuple[KnownMimeType, str]: | ||
if html.url is not None: | ||
# TODO(akshayka): resize iframe not working | ||
data = builder.h.iframe( | ||
src=html.url, | ||
width="100%", | ||
onload="__resizeIframe(this)", | ||
scrolling="auto", | ||
frameborder="0", | ||
) | ||
else: | ||
data = str(html._repr_html_()) # type: ignore | ||
|
||
return ("text/html", data) | ||
|
||
return unpatch |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
# Copyright 2024 Marimo. All rights reserved. | ||
import marimo | ||
|
||
__generated_with = "0.3.1" | ||
app = marimo.App() | ||
|
||
|
||
@app.cell | ||
def __(): | ||
import IPython | ||
import marimo as mo | ||
|
||
url = IPython.display.HTML("https://marimo.io") | ||
url | ||
return IPython, mo, url | ||
|
||
|
||
@app.cell | ||
def __(IPython): | ||
html = IPython.display.HTML("<em>hello world</em>") | ||
html | ||
return html, | ||
|
||
|
||
@app.cell | ||
def __(IPython, html, url): | ||
IPython.display.display(html, url) | ||
return | ||
|
||
|
||
@app.cell | ||
def __(): | ||
# not on PyPI | ||
# installation instructions here https://github.com/allefeld/pytikz | ||
import tikz | ||
return tikz, | ||
|
||
|
||
@app.cell | ||
def __(tikz): | ||
# define coordinates as a list of tuples | ||
coords = [(0, 0), (0, 2), (1, 3.25), (2, 2), (2, 0), (0, 2), (2, 2), (0, 0), (2, 0)] | ||
|
||
# create `Picture` object | ||
pic = tikz.Picture() | ||
# draw a line following the coordinates | ||
pic.draw(tikz.line(coords), thick=True, rounded_corners='4pt') | ||
return coords, pic | ||
|
||
|
||
@app.cell | ||
def __(pic): | ||
pic.demo(dpi=300) | ||
return | ||
|
||
|
||
if __name__ == "__main__": | ||
app.run() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,6 +113,7 @@ testoptional = [ | |
# have the rust toolchain installed on CI | ||
"polars==0.19.12", | ||
"anywidget~=0.9.3", | ||
"ipython~=8.12.3", | ||
"openai~=1.12.0", | ||
] | ||
|
||
|
@@ -194,6 +195,7 @@ exclude = [ | |
'marimo/_tutorials/', | ||
'marimo/_smoke_tests/', | ||
] | ||
warn_unused_ignores=false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahhhhh! this was the worst without this since you couldn't do a blanket ignore b/c in another test matrix it would fail. and other matrices would have different ignores.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. haha yea it was getting really cumbersome |
||
|
||
# tutorials shouldn't be type-checked (should be excluded), but they | ||
# get included anyway, maybe due to import following; this is coarse but works | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
from __future__ import annotations | ||
|
||
import importlib | ||
import os.path | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this wont work for CORS - not sure if thats what you were testing, but that would be once reason.
if you do same origin, it should work. you could make marimo a proxy for URLs (but i think that could be done later)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah i see. will put off for later. embedding urls is likely not a common use case