Skip to content

Commit

Permalink
Do not require | None when defining Renderers
Browse files Browse the repository at this point in the history
Fixes #1021
  • Loading branch information
schloerke committed Jan 17, 2024
1 parent 3797a5d commit 4decf0a
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 22 deletions.
26 changes: 16 additions & 10 deletions shiny/render/renderer/_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,12 @@
DefaultUIFnResultOrNone = Union[DefaultUIFnResult, None]
DefaultUIFn = Callable[[str], DefaultUIFnResultOrNone]

# Requiring `None` type throughout the value functions as `return` returns `None` type.
# This is typically paired with `req(False)` to exit quickly.
# If package authors want to NOT allow `None` type, they can capture it in a custom render method with a runtime error. (Or make a new RendererThatCantBeNone class)
ValueFn = Union[
Callable[[], IT],
Callable[[], Awaitable[IT]],
Callable[[], Union[IT, None]],
Callable[[], Awaitable[Union[IT, None]]],
]
"""
App-supplied output value function which returns type `IT`. This function can be
Expand Down Expand Up @@ -223,7 +226,10 @@ class AsyncValueFn(Generic[IT]):
Type definition: `Callable[[], Awaitable[IT]]`
"""

def __init__(self, fn: Callable[[], IT] | Callable[[], Awaitable[IT]]):
def __init__(
self,
fn: Callable[[], IT | None] | Callable[[], Awaitable[IT | None]],
):
if isinstance(fn, AsyncValueFn):
raise TypeError(
"Must not call `AsyncValueFn.__init__` with an object of class `AsyncValueFn`"
Expand All @@ -232,7 +238,7 @@ def __init__(self, fn: Callable[[], IT] | Callable[[], Awaitable[IT]]):
self._fn = wrap_async(fn)
self._orig_fn = fn

async def __call__(self) -> IT:
async def __call__(self) -> IT | None:
"""
Call the asynchronous function.
"""
Expand All @@ -249,7 +255,7 @@ def is_async(self) -> bool:
"""
return self._is_async

def get_async_fn(self) -> Callable[[], Awaitable[IT]]:
def get_async_fn(self) -> Callable[[], Awaitable[IT | None]]:
"""
Return the async value function.
Expand All @@ -260,7 +266,7 @@ def get_async_fn(self) -> Callable[[], Awaitable[IT]]:
"""
return self._fn

def get_sync_fn(self) -> Callable[[], IT]:
def get_sync_fn(self) -> Callable[[], IT | None]:
"""
Retrieve the original, synchronous value function function.
Expand All @@ -286,14 +292,14 @@ class Renderer(RendererBase, Generic[IT]):
TODO-barret-docs
"""

fn: AsyncValueFn[IT | None]
fn: AsyncValueFn[IT]
"""
App-supplied output value function which returns type `IT`. This function is always
asyncronous as the original app-supplied function possibly wrapped to execute
asynchonously.
"""

def __call__(self, _fn: ValueFn[IT | None]) -> Self:
def __call__(self, _fn: ValueFn[IT]) -> Self:
"""
Renderer __call__ docs here; Sets app's value function
Expand All @@ -308,7 +314,7 @@ def __call__(self, _fn: ValueFn[IT | None]) -> Self:
self.__name__: str = _fn.__name__

# Set value function with extra meta information
self.fn: AsyncValueFn[IT | None] = AsyncValueFn(_fn)
self.fn: AsyncValueFn[IT] = AsyncValueFn(_fn)

# Allow for App authors to not require `@output`
self._auto_register()
Expand All @@ -317,7 +323,7 @@ def __call__(self, _fn: ValueFn[IT | None]) -> Self:

def __init__(
self,
_fn: Optional[ValueFn[IT | None]] = None,
_fn: Optional[ValueFn[IT]] = None,
):
# Do not display docs here. If docs are present, it could highjack the docs of
# the subclass's `__init__` method.
Expand Down
24 changes: 12 additions & 12 deletions shiny/render/transformer/_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
overload,
)

from ..renderer import AsyncValueFn, Jsonifiable, RendererBase
from ..renderer import Jsonifiable, RendererBase
from ..renderer._renderer import DefaultUIFn, DefaultUIFnResultOrNone

if TYPE_CHECKING:
Expand Down Expand Up @@ -255,13 +255,16 @@ def __init__(
" Ex `async def my_transformer(....`"
)

# Upgrade value function to be async;
# Calling an async function has a ~35ns overhead (barret's machine)
# Checking if a function is async has a 180+ns overhead (barret's machine)
# -> It is faster to always call an async function than to always check if it is async
# Always being async simplifies the execution
self._fn = AsyncValueFn(value_fn)
self._value_fn_is_async = self._fn.is_async() # legacy key
# # Upgrade value function to be async;
# # Calling an async function has a ~35ns overhead (barret's machine)
# # Checking if a function is async has a 180+ns overhead (barret's machine)
# # -> It is faster to always call an async function than to always check if it is async
# # Always being async simplifies the execution
# # Not used
# self._fn = AsyncValueFn(value_fn)

self._value_fn = value_fn
self._value_fn_is_async = is_async_callable(value_fn) # legacy key
self.__name__ = value_fn.__name__

self._transformer = transform_fn
Expand Down Expand Up @@ -302,14 +305,11 @@ async def _run(self) -> OT:
`*args` is required to use with `**kwargs` when using
`typing.ParamSpec`.
"""
value_fn = (
self._fn.get_async_fn() if self._fn.is_async() else self._fn.get_sync_fn()
)
ret = await self._transformer(
# TransformerMetadata
self._meta(),
# Callable[[], IT] | Callable[[], Awaitable[IT]]
value_fn,
self._value_fn,
# P
*self._params.args,
**self._params.kwargs,
Expand Down

0 comments on commit 4decf0a

Please sign in to comment.