Skip to content
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
36 changes: 20 additions & 16 deletions shiny/render/_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
from .. import _utils
from .. import ui as _ui
from .._namespaces import ResolvedId
from ..types import ImgData
from ..types import MISSING, MISSING_TYPE, ImgData
from ._try_render_plot import (
PlotSizeInfo,
try_render_matplotlib,
Expand Down Expand Up @@ -115,8 +115,8 @@ async def PlotTransformer(
_fn: ValueFn[object],
*,
alt: Optional[str] = None,
width: Optional[float] = None,
height: Optional[float] = None,
width: float | None | MISSING_TYPE = MISSING,
height: float | None | MISSING_TYPE = MISSING,
**kwargs: object,
) -> ImgData | None:
is_userfn_async = is_async_callable(_fn)
Expand All @@ -142,12 +142,16 @@ def container_size(dimension: Literal["width", "height"]) -> float:
result = inputs[ResolvedId(f".clientdata_output_{name}_{dimension}")]()
return typing.cast(float, result)

non_missing_size = (
cast(Union[float, None], width) if width is not MISSING else None,
cast(Union[float, None], height) if height is not MISSING else None,
)
plot_size_info = PlotSizeInfo(
container_size_px_fn=(
lambda: container_size("width"),
lambda: container_size("height"),
),
user_specified_size_px=(width, height),
user_specified_size_px=non_missing_size,
pixelratio=pixelratio,
)

Expand Down Expand Up @@ -218,8 +222,8 @@ def container_size(dimension: Literal["width", "height"]) -> float:
def plot(
*,
alt: Optional[str] = None,
width: Optional[float] = None,
height: Optional[float] = None,
width: float | None | MISSING_TYPE = MISSING,
height: float | None | MISSING_TYPE = MISSING,
**kwargs: Any,
) -> PlotTransformer.OutputRendererDecorator:
...
Expand All @@ -234,8 +238,8 @@ def plot(
_fn: PlotTransformer.ValueFn | None = None,
*,
alt: Optional[str] = None,
width: Optional[float] = None,
height: Optional[float] = None,
width: float | None | MISSING_TYPE = MISSING,
height: float | None | MISSING_TYPE = MISSING,
**kwargs: Any,
) -> PlotTransformer.OutputRenderer | PlotTransformer.OutputRendererDecorator:
"""
Expand All @@ -247,15 +251,15 @@ def plot(
Alternative text for the image if it cannot be displayed or viewed (i.e., the
user uses a screen reader).
width
Width of the plot in pixels. If ``None``, the width will be determined by the
size of the corresponding :func:`~shiny.ui.output_plot`. (You should not need to
use this argument in most Shiny apps--set the desired width on
:func:`~shiny.ui.output_plot` instead.)
Width of the plot in pixels. If ``None`` or ``MISSING``, the width will be
determined by the size of the corresponding :func:`~shiny.ui.output_plot`. (You
should not need to use this argument in most Shiny apps--set the desired width
on :func:`~shiny.ui.output_plot` instead.)
height
Height of the plot in pixels. If ``None``, the height will be determined by the
size of the corresponding :func:`~shiny.ui.output_plot`. (You should not need to
use this argument in most Shiny apps--set the desired height on
:func:`~shiny.ui.output_plot` instead.)
Height of the plot in pixels. If ``None`` or ``MISSING``, the height will be
determined by the size of the corresponding :func:`~shiny.ui.output_plot`. (You
should not need to use this argument in most Shiny apps--set the desired height
on :func:`~shiny.ui.output_plot` instead.)
**kwargs
Additional keyword arguments passed to the relevant method for saving the image
(e.g., for matplotlib, arguments to ``savefig()``; for PIL and plotnine,
Expand Down
37 changes: 5 additions & 32 deletions shiny/render/transformer/_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
from ..._docstring import add_example
from ..._typing_extensions import Concatenate, ParamSpec
from ..._utils import is_async_callable, run_coro_sync
from ...types import MISSING

# Input type for the user-spplied function that is passed to a render.xx
IT = TypeVar("IT")
Expand Down Expand Up @@ -353,10 +354,12 @@ def _render_default(self) -> TagList | Tag | MetadataNode | str:
{
k: v
for k, v in self._params.kwargs.items()
if k in self.default_ui_passthrough_args
if k in self.default_ui_passthrough_args and v is not MISSING
}
)
kwargs.update(self.default_ui_kwargs)
kwargs.update(
{k: v for k, v in self.default_ui_kwargs.items() if v is not MISSING}
)
return cast(DefaultUIFn, self.default_ui)(
self.__name__, *self.default_ui_args, **kwargs
)
Expand Down Expand Up @@ -800,33 +803,3 @@ async def resolve_value_fn(value_fn: ValueFn[IT]) -> IT:


R = TypeVar("R")


def decorator_args_passthrough(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was an unused function, from an older implementation that I moved away from after @wch code review feedback.

fn: Callable[P, R], passthrough: tuple[str, ...]
) -> Callable[Concatenate[dict[str, object], P], R]:
"""
Modifies a default_ui function so that it can receive certain kwargs that are
passed to the OutputRenderer. For example, @render.plot takes `width` and `height`
arguments that we'd like to pass through to `ui.output_plot`. We can do that by
calling `enrich_default_ui_output_fn(ui.output_plot, ("width", "height"))`.

This works by returning a wrapped version of the function that has the same
signature, except, with an added `_params` first argument. The OutputRenderer base
class will look for this magic argument and pass it the params it was created with.
The wrapped function will use those to prepopulate the kwargs.
"""

def inner(_params: dict[str, object], *args: P.args, **kwargs: P.kwargs):
# Filter down to just the params that we care about (`in passthrough`) and did
# not explicitly get overwritten by more explicit argument passing (`in kwargs`)
extra_args = {
k: v for k, v in _params.items() if k in passthrough and k not in kwargs
}
# Theoretically this is mutating a dict that possibly doesn't belong to us. In
# practice, we're the only ones to call default_ui, so it's fine.
kwargs.update(extra_args)
# Call the wrapped function and return the result
return fn(*args, **kwargs)

return inner
13 changes: 0 additions & 13 deletions tests/e2e/plot-sizing/test_plot_sizing.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,3 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None:
tolerance += 2
assert abs(rect["width"] - 300) <= tolerance
assert abs(rect["height"] - 200) <= tolerance


def test_decorator_passthrough_size():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved this from e2e test to regular pytest, since it didn't actually use any playwright logic.

"""Make sure that render.plot width/height are passed through to implicit output"""
from shiny import render

@render.plot(width=1280, height=960)
def foo():
...

rendered = str(foo.tagify())
assert "1280px" in rendered
assert "960px" in rendered
64 changes: 64 additions & 0 deletions tests/pytest/test_plot_sizing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
from shiny import render, ui
from shiny.express import output_args
from shiny.types import MISSING


def test_decorator_plot_sizing():
"""render.plot width/height are passed through to implicit output"""

@render.plot(width=1280, height=960)
def foo():
...

rendered = str(foo.tagify())
assert "1280px" in rendered
assert "960px" in rendered
assert rendered == str(ui.output_plot("foo", width=1280, height=960))


def test_decorator_plot_default():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the test case that reflects the issue the PR is meant to fix.

"""render.plot default is the same as ui.output_plot default"""

@render.plot()
def foo():
...

rendered = str(foo.tagify())
assert rendered == str(ui.output_plot("foo"))


def test_decorator_output_args():
"""@output_args is respected"""

@output_args(width="640px", height="480px")
@render.plot()
def foo():
...

rendered = str(foo.tagify())
assert rendered == str(ui.output_plot("foo", width="640px", height="480px"))


def test_decorator_output_args_priority():
"""@output_args should override render.plot width/height"""

@output_args(width="640px", height=480)
@render.plot(width=1280, height=960)
def foo():
...

rendered = str(foo.tagify())
# Note "640px" => 640 and 480 => "480px"
assert rendered == str(ui.output_plot("foo", width=640, height="480px"))


def test_decorator_output_args_MISSING():
"""Not saying we support this, but test how MISSING interacts"""

@output_args(width=MISSING)
@render.plot(width=1280, height=MISSING)
def foo():
...

rendered = str(foo.tagify())
assert rendered == str(ui.output_plot("foo", width="1280px"))