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

improvement: patch IPython.display.display to call mo.output.append #919

Merged
merged 5 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions marimo/_dependencies/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,8 @@ def has_anywidget() -> bool:
def has_watchdog() -> bool:
"""Return True if watchdog is installed."""
return importlib.util.find_spec("watchdog") is not None

@staticmethod
def has_ipython() -> bool:
"""Return True if IPython is installed."""
return importlib.util.find_spec("IPython") is not None
3 changes: 3 additions & 0 deletions marimo/_output/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def iframe(
height: Optional[str] = None,
style: Optional[str] = None,
onload: Optional[str] = None,
**kwargs: str,
) -> str:
params: List[Tuple[str, Union[str, None]]] = []
if src:
Expand All @@ -107,6 +108,8 @@ def iframe(
params.append(("style", style))
if onload:
params.append(("onload", onload))
for key, value in kwargs.items():
params.append((key, value))

if len(params) == 0:
return "<iframe />"
Expand Down
7 changes: 5 additions & 2 deletions marimo/_output/formatters/formatter_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from __future__ import annotations

import abc
from typing import Optional
from typing import Callable, Optional


# Abstract base class for formatters that are installed at runtime.
Expand All @@ -20,9 +20,12 @@ def package_name() -> Optional[str]:
raise NotImplementedError

@abc.abstractmethod
def register(self) -> None:
def register(self) -> Callable[[], None] | None:
"""Registers formatters.

Formatters can be registered using the formatters.formatter decorator.

Optionally returns a handle to undo side-effects, such as module
patches.
"""
raise NotImplementedError
2 changes: 2 additions & 0 deletions marimo/_output/formatters/formatters.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from marimo._output.formatters.cell import CellFormatter
from marimo._output.formatters.formatter_factory import FormatterFactory
from marimo._output.formatters.holoviews_formatters import HoloViewsFormatter
from marimo._output.formatters.ipython_formatters import IPythonFormatter
from marimo._output.formatters.leafmap_formatters import LeafmapFormatter
from marimo._output.formatters.matplotlib_formatters import MatplotlibFormatter
from marimo._output.formatters.pandas_formatters import PandasFormatter
Expand All @@ -29,6 +30,7 @@
LeafmapFormatter.package_name(): LeafmapFormatter(),
BokehFormatter.package_name(): BokehFormatter(),
HoloViewsFormatter.package_name(): HoloViewsFormatter(),
IPythonFormatter.package_name(): IPythonFormatter(),
AnyWidgetFormatter.package_name(): AnyWidgetFormatter(),
}

Expand Down
60 changes: 60 additions & 0 deletions marimo/_output/formatters/ipython_formatters.py
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
Copy link
Contributor

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)

Copy link
Contributor Author

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

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
2 changes: 1 addition & 1 deletion marimo/_plugins/ui/_impl/dataframes/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def handle_aggregate(
# Pandas type-checking doesn't like the fact that the values
# are lists of strings (function names), even though the docs permit
# such a value
return cast("pd.DataFrame", df.agg(dict_of_aggs)) # type: ignore[arg-type] # noqa: E501
return cast("pd.DataFrame", df.agg(dict_of_aggs)) # type: ignore # noqa: E501

@staticmethod
def handle_select_columns(
Expand Down
58 changes: 58 additions & 0 deletions marimo/_smoke_tests/third_party/ipython_display.py
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()
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]

Expand Down Expand Up @@ -194,6 +195,7 @@ exclude = [
'marimo/_tutorials/',
'marimo/_smoke_tests/',
]
warn_unused_ignores=false
Copy link
Contributor

Choose a reason for hiding this comment

The 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..

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down
2 changes: 2 additions & 0 deletions tests/_output/formatters/test_formatters.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import importlib
import os.path

Expand Down
5 changes: 1 addition & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@
ThreadSafeStdout,
ThreadSafeStream,
)
from marimo._runtime.context import (
initialize_context,
teardown_context,
)
from marimo._runtime.context import initialize_context, teardown_context
from marimo._runtime.requests import AppMetadata, ExecutionRequest
from marimo._runtime.runtime import Kernel

Expand Down
Loading