From 9c350a24e3476df4aa375fe14ede4d7379e733b0 Mon Sep 17 00:00:00 2001 From: Carson Date: Tue, 8 Mar 2022 11:09:05 -0600 Subject: [PATCH 01/13] wip reduce need to explicitly namespace Module() UI code --- examples/moduleapp/app.py | 10 +-- shiny/_modules.py | 123 ++++++++----------------------- shiny/_namespaces.py | 34 +++++++++ shiny/examples/Module/app.py | 10 +-- shiny/session/_session.py | 24 ++++-- shiny/ui/_download_button.py | 5 +- shiny/ui/_input_action_button.py | 12 ++- shiny/ui/_input_check_radio.py | 13 ++-- shiny/ui/_input_date.py | 11 +-- shiny/ui/_input_file.py | 3 +- shiny/ui/_input_numeric.py | 3 +- shiny/ui/_input_password.py | 3 +- shiny/ui/_input_select.py | 3 +- shiny/ui/_input_slider.py | 3 + shiny/ui/_input_text.py | 5 +- shiny/ui/_input_update.py | 11 ++- shiny/ui/_navs.py | 15 ++-- shiny/ui/_output.py | 11 +-- shiny/ui/_page.py | 3 +- tests/test_modules.py | 22 +++--- 20 files changed, 165 insertions(+), 159 deletions(-) create mode 100644 shiny/_namespaces.py diff --git a/examples/moduleapp/app.py b/examples/moduleapp/app.py index e63150b77..cd785247f 100644 --- a/examples/moduleapp/app.py +++ b/examples/moduleapp/app.py @@ -1,18 +1,14 @@ -from typing import Callable from shiny import * - # ============================================================ # Counter module # ============================================================ -def counter_ui( - ns: Callable[[str], str], label: str = "Increment counter" -) -> ui.TagChildArg: +def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: return ui.div( {"style": "border: 1px solid #ccc; border-radius: 5px; margin: 5px 0;"}, ui.h2("This is " + label), - ui.input_action_button(id=ns("button"), label=label), - ui.output_text_verbatim(id=ns("out")), + ui.input_action_button(id="button", label=label), + ui.output_text_verbatim(id="out"), ) diff --git a/shiny/_modules.py b/shiny/_modules.py index 77c6424f6..638ddcf0a 100644 --- a/shiny/_modules.py +++ b/shiny/_modules.py @@ -1,18 +1,17 @@ -__all__ = ("Module",) +__all__ = ("Module", "namespaced_id") -from typing import Any, Callable, Optional, Union +from typing import Any, Callable, Optional from htmltools import TagChildArg from ._docstring import add_example -from .reactive import Value -from .render import RenderFunction +from ._namespaces import namespaced_id from .session import Inputs, Outputs, Session, require_active_session, session_context class ModuleInputs(Inputs): """ - A class representing the inputs of a module. + A class representing a module's outputs. Warning ------- @@ -23,43 +22,16 @@ class ModuleInputs(Inputs): signature as :class:`shiny.session.Session`. """ - def __init__(self, ns: str, values: Inputs): - self._ns: str = ns - self._values: Inputs = values - - def _ns_key(self, key: str) -> str: - return self._ns + "-" + key - - def __setitem__(self, key: str, value: Value[Any]) -> None: - self._values[self._ns_key(key)].set(value) - - def __getitem__(self, key: str) -> Value[Any]: - return self._values[self._ns_key(key)] - - def __delitem__(self, key: str) -> None: - del self._values[self._ns_key(key)] - - # Allow access of values as attributes. - def __setattr__(self, attr: str, value: Value[Any]) -> None: - if attr in ("_values", "_ns", "_ns_key"): - object.__setattr__(self, attr, value) - return - else: - self.__setitem__(attr, value) - - def __getattr__(self, attr: str) -> Value[Any]: - if attr in ("_values", "_ns", "_ns_key"): - return object.__getattribute__(self, attr) - else: - return self.__getitem__(attr) - - def __delattr__(self, key: str) -> None: - self.__delitem__(key) + def __init__(self, ns: str, parent_inputs: Inputs): + self._ns = namespaced_id(ns, parent_inputs._ns) # Support nested modules + # Don't set _parent attribute like the other classes since Inputs redefines + # __setattr__ + self._map = parent_inputs._map class ModuleOutputs(Outputs): """ - A class representing the outputs of a module. + A class representing a module's outputs. Warning ------- @@ -70,49 +42,17 @@ class ModuleOutputs(Outputs): signature as :class:`shiny.session.Session`. """ - def __init__(self, ns: str, outputs: Outputs): - self._ns: str = ns - self._outputs: Outputs = outputs - - def _ns_key(self, key: str) -> str: - return self._ns + "-" + key + def __init__(self, ns: str, parent_outputs: Outputs): + self._ns = namespaced_id(ns, parent_outputs._ns) # Support nested modules + self._parent = parent_outputs - def __call__( - self, - fn: Optional[RenderFunction] = None, - *, - id: Optional[str] = None, - suspend_when_hidden: bool = True, - priority: int = 0, - name: Optional[str] = None, - ) -> Union[None, Callable[[RenderFunction], None]]: - if name is not None: - from . import _deprecated - - _deprecated.warn_deprecated( - "`@output(name=...)` is deprecated. Use `@output(id=...)` instead." - ) - id = name - - def set_fn(fn: RenderFunction) -> None: - output_id = id or fn.__name__ - output_id = self._ns_key(output_id) - out_fn = self._outputs( - id=output_id, - suspend_when_hidden=suspend_when_hidden, - priority=priority, - ) - return out_fn(fn) - - if fn is None: - return set_fn - else: - return set_fn(fn) + def __getattr__(self, attr: str) -> Any: + return getattr(self._parent, attr) class ModuleSession(Session): """ - A class representing the session of a module. + A class representing a module's outputs. Warning ------- @@ -123,8 +63,8 @@ class ModuleSession(Session): signature as :class:`shiny.session.Session`. """ - def __init__(self, ns: str, parent_session: Session) -> None: - self._ns: str = ns + def __init__(self, ns: str, parent_session: Session): + self._ns: str = namespaced_id(ns, parent_session._ns) # Support nested modules self._parent: Session = parent_session self.input: ModuleInputs = ModuleInputs(ns, parent_session.input) self.output: ModuleOutputs = ModuleOutputs(ns, parent_session.output) @@ -133,6 +73,11 @@ def __getattr__(self, attr: str) -> Any: return getattr(self._parent, attr) +class MockModuleSession(ModuleSession): + def __init__(self, ns: str): + self._ns = ns + + @add_example() class Module: """ @@ -167,7 +112,11 @@ def ui(self, ns: str, *args: Any) -> TagChildArg: args Additional arguments to pass to the module's UI definition. """ - return self._ui(Module._make_ns_fn(ns), *args) + + # Create a fake session so that namespaced_id() knows + # what the relevant namespace is + with session_context(MockModuleSession(ns)): + return self._ui(*args) def server(self, ns: str, *, session: Optional[Session] = None) -> None: """ @@ -181,15 +130,7 @@ def server(self, ns: str, *, session: Optional[Session] = None) -> None: A :class:`~shiny.Session` instance. If not provided, it is inferred via :func:`~shiny.session.get_current_session`. """ - self.ns: str = ns - session = require_active_session(session) - session_proxy = ModuleSession(ns, session) - with session_context(session_proxy): - self._server(session_proxy.input, session_proxy.output, session_proxy) - - @staticmethod - def _make_ns_fn(namespace: str) -> Callable[[str], str]: - def ns_fn(id: str) -> str: - return namespace + "-" + id - - return ns_fn + + mod_sess = ModuleSession(ns, require_active_session(session)) + with session_context(mod_sess): + return self._server(mod_sess.input, mod_sess.output, mod_sess) diff --git a/shiny/_namespaces.py b/shiny/_namespaces.py new file mode 100644 index 000000000..c6c876167 --- /dev/null +++ b/shiny/_namespaces.py @@ -0,0 +1,34 @@ +# TODO: make this available under the shiny.modules API +__all__ = ("namespaced_id",) + +from typing import Union, Optional + +from .types import MISSING, MISSING_TYPE + + +def namespaced_id(id: str, ns: Union[str, MISSING_TYPE, None] = MISSING) -> str: + """ + Namespace an ID based on the current ``Module()``'s namespace. + + Parameters + ---------- + id + The ID to namespace.. + """ + if isinstance(ns, MISSING_TYPE): + ns = get_current_namespace() + + if ns is None: + return id + else: + return ns + "_" + id + + +def get_current_namespace() -> Optional[str]: + from .session import get_current_session + + session = get_current_session() + if session is None: + return None + else: + return session._ns diff --git a/shiny/examples/Module/app.py b/shiny/examples/Module/app.py index e63150b77..cd785247f 100644 --- a/shiny/examples/Module/app.py +++ b/shiny/examples/Module/app.py @@ -1,18 +1,14 @@ -from typing import Callable from shiny import * - # ============================================================ # Counter module # ============================================================ -def counter_ui( - ns: Callable[[str], str], label: str = "Increment counter" -) -> ui.TagChildArg: +def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: return ui.div( {"style": "border: 1px solid #ccc; border-radius: 5px; margin: 5px 0;"}, ui.h2("This is " + label), - ui.input_action_button(id=ns("button"), label=label), - ui.output_text_verbatim(id=ns("out")), + ui.input_action_button(id="button", label=label), + ui.output_text_verbatim(id="out"), ) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index ea426199b..913f00615 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -49,6 +49,7 @@ from .._fileupload import FileInfo, FileUploadManager from ..http_staticfiles import FileResponse from ..input_handler import input_handlers +from .._namespaces import namespaced_id from ..reactive import Value, Effect, Effect_, isolate, flush from ..reactive._core import lock from ..types import SafeException, SilentCancelOutputException, SilentException @@ -141,6 +142,8 @@ def __init__( self.input: Inputs = Inputs() self.output: Outputs = Outputs(self) + self._ns: Optional[str] = None # Only relevant for ModuleSession + self.user: Union[str, None] = None self.groups: Union[List[str], None] = None credentials_json: str = "" @@ -336,6 +339,7 @@ async def uploadEnd(job_id: str, input_id: str) -> None: # ========================================================================== # Handling /session/{session_id}/{action}/{subpath} requests # ========================================================================== + # TODO: anything to be done here for module support? async def _handle_request( self, request: Request, action: str, subpath: Optional[str] ) -> ASGIApp: @@ -471,7 +475,7 @@ def send_input_message(self, id: str, message: Dict[str, object]) -> None: message The message to send. """ - msg: Dict[str, object] = {"id": id, "message": message} + msg: Dict[str, object] = {"id": namespaced_id(id, self._ns), "message": message} self._outbound_message_queues["input_messages"].append(msg) self._request_flush() @@ -639,6 +643,7 @@ async def _unhandled_error(self, e: Exception) -> None: await self.close() # TODO: probably name should be id + # TODO: anything to be done here for module support? @add_example() def download( self, @@ -748,13 +753,16 @@ def __init__(self, **kwargs: object) -> None: for key, value in kwargs.items(): self._map[key] = Value(value, read_only=True) + self._ns: Optional[str] = None # Only relevant for ModuleInputs() + def __setitem__(self, key: str, value: Value[Any]) -> None: if not isinstance(value, Value): raise TypeError("`value` must be a reactive.Value object.") - self._map[key] = value + self._map[namespaced_id(key, self._ns)] = value def __getitem__(self, key: str) -> Value[Any]: + key = namespaced_id(key, self._ns) # Auto-populate key if accessed but not yet set. Needed to take reactive # dependencies on input values that haven't been received from client # yet. @@ -764,19 +772,18 @@ def __getitem__(self, key: str) -> Value[Any]: return self._map[key] def __delitem__(self, key: str) -> None: - del self._map[key] + del self._map[namespaced_id(key, self._ns)] # Allow access of values as attributes. def __setattr__(self, attr: str, value: Value[Any]) -> None: - # Need special handling of "_map". - if attr == "_map": + if attr in ("_map", "_ns"): super().__setattr__(attr, value) return self.__setitem__(attr, value) def __getattr__(self, attr: str) -> Value[Any]: - if attr == "_map": + if attr in ("_map", "_ns"): return object.__getattribute__(self, attr) return self.__getitem__(attr) @@ -803,6 +810,7 @@ def __init__(self, session: Session) -> None: self._effects: Dict[str, Effect_] = {} self._suspend_when_hidden: Dict[str, bool] = {} self._session: Session = session + self._ns: Optional[str] = None # Only relevant for ModuleOutputs() @overload def __call__(self, fn: render.RenderFunction) -> None: @@ -837,7 +845,9 @@ def __call__( id = name def set_fn(fn: render.RenderFunction) -> None: - output_name = id or fn.__name__ + # Get the (possibly namespaced) output id + output_name = namespaced_id(id or fn.__name__, self._ns) + # fn is either a regular function or a RenderFunction object. If # it's the latter, give it a bit of metadata. if isinstance(fn, render.RenderFunction): diff --git a/shiny/ui/_download_button.py b/shiny/ui/_download_button.py index fceb14756..70d32c14c 100644 --- a/shiny/ui/_download_button.py +++ b/shiny/ui/_download_button.py @@ -5,6 +5,7 @@ from htmltools import tags, Tag, TagChildArg, TagAttrArg, css from .._docstring import add_example +from .._namespaces import namespaced_id from .._shinyenv import is_pyodide @@ -46,7 +47,7 @@ def download_button( icon, label, {"class": "btn btn-default shiny-download-link", "style": css(width=width)}, - id=id, + id=namespaced_id(id), # This is a fake link that just results in a 404. It will be replaced by a # working link after the server side logic runs, so this link will only be # visited in cases where the user clicks the button too fast, or if the server @@ -99,7 +100,7 @@ def download_link( icon, label, {"class": "shiny-download-link", "style": css(width=width)}, - id=id, + id=namespaced_id(id), href="session/0/download/missing_download", target="_blank", download=None if is_pyodide else True, diff --git a/shiny/ui/_input_action_button.py b/shiny/ui/_input_action_button.py index 554ee138f..6bbe2705c 100644 --- a/shiny/ui/_input_action_button.py +++ b/shiny/ui/_input_action_button.py @@ -5,6 +5,7 @@ from htmltools import tags, Tag, TagChildArg, TagAttrArg, css from .._docstring import add_example +from .._namespaces import namespaced_id @add_example() @@ -53,7 +54,7 @@ def input_action_button( {"class": "btn btn-default action-button", "style": css(width=width)}, icon, label, - id=id, + id=namespaced_id(id), type="button", **kwargs, ) @@ -98,4 +99,11 @@ def input_action_link( ~shiny.event """ - return tags.a({"class": "action-button"}, icon, label, id=id, href="#", **kwargs) + return tags.a( + {"class": "action-button"}, + icon, + label, + id=namespaced_id(id), + href="#", + **kwargs, + ) diff --git a/shiny/ui/_input_check_radio.py b/shiny/ui/_input_check_radio.py index 2b312a9ad..87b2f4b44 100644 --- a/shiny/ui/_input_check_radio.py +++ b/shiny/ui/_input_check_radio.py @@ -10,6 +10,7 @@ from .._docstring import add_example +from .._namespaces import namespaced_id from ._utils import shiny_input_label # Canonical format for representing select options. @@ -58,7 +59,9 @@ def input_checkbox( div( tags.label( tags.input( - id=id, type="checkbox", checked="checked" if value else None + id=namespaced_id(id), + type="checkbox", + checked="checked" if value else None, ), span(label), ), @@ -119,7 +122,7 @@ def input_checkbox_group( input_label = shiny_input_label(id, label) options = _generate_options( - id=id, + id=namespaced_id(id), type="checkbox", choices=choices, selected=selected, @@ -128,7 +131,7 @@ def input_checkbox_group( return div( input_label, options, - id=id, + id=namespaced_id(id), style=css(width=width), class_="form-group shiny-input-checkboxgroup shiny-input-container" + (" shiny-input-container-inline" if inline else ""), @@ -186,7 +189,7 @@ def input_radio_buttons( input_label = shiny_input_label(id, label) options = _generate_options( - id=id, + id=namespaced_id(id), type="radio", choices=choices, selected=selected, @@ -195,7 +198,7 @@ def input_radio_buttons( return div( input_label, options, - id=id, + id=namespaced_id(id), style=css(width=width), class_="form-group shiny-input-radiogroup shiny-input-container" + (" shiny-input-container-inline" if inline else ""), diff --git a/shiny/ui/_input_date.py b/shiny/ui/_input_date.py index e5a53a350..94b538eaf 100644 --- a/shiny/ui/_input_date.py +++ b/shiny/ui/_input_date.py @@ -8,6 +8,7 @@ from .._docstring import add_example from ._html_dependencies import datepicker_deps +from .._namespaces import namespaced_id from ._utils import shiny_input_label @@ -109,7 +110,7 @@ def input_date( return div( shiny_input_label(id, label), _date_input_tag( - id=id, + id=namespaced_id(id), value=value, min=min, max=max, @@ -121,7 +122,7 @@ def input_date( data_date_dates_disabled=json.dumps(datesdisabled), data_date_days_of_week_disabled=json.dumps(daysofweekdisabled), ), - id=id, + id=namespaced_id(id), class_="shiny-date-input form-group shiny-input-container", style=css(width=width), ) @@ -227,7 +228,7 @@ def input_date_range( shiny_input_label(id, label), div( _date_input_tag( - id=id, + id=namespaced_id(id), value=start, min=min, max=max, @@ -243,7 +244,7 @@ def input_date_range( class_="input-group-addon input-group-prepend input-group-append", ), _date_input_tag( - id=id, + id=namespaced_id(id), value=end, min=min, max=max, @@ -256,7 +257,7 @@ def input_date_range( # input-daterange class is needed for dropdown behavior class_="input-daterange input-group input-group-sm", ), - id=id, + id=namespaced_id(id), class_="shiny-date-range-input form-group shiny-input-container", style=css(width=width), ) diff --git a/shiny/ui/_input_file.py b/shiny/ui/_input_file.py index 2a43bf917..62d096207 100644 --- a/shiny/ui/_input_file.py +++ b/shiny/ui/_input_file.py @@ -11,6 +11,7 @@ from htmltools import Tag, TagChildArg, css, div, span, tags from .._docstring import add_example +from .._namespaces import namespaced_id from ._utils import shiny_input_label @@ -89,7 +90,7 @@ def input_file( btn_file = span( button_label, tags.input( - id=id, + id=namespaced_id(id), name=id, type="file", multiple="multiple" if multiple else None, diff --git a/shiny/ui/_input_numeric.py b/shiny/ui/_input_numeric.py index 5fc60f3f5..ee3444fdc 100644 --- a/shiny/ui/_input_numeric.py +++ b/shiny/ui/_input_numeric.py @@ -5,6 +5,7 @@ from htmltools import tags, Tag, div, css, TagChildArg from .._docstring import add_example +from .._namespaces import namespaced_id from ._utils import shiny_input_label @@ -57,7 +58,7 @@ def input_numeric( return div( shiny_input_label(id, label), tags.input( - id=id, + id=namespaced_id(id), type="number", class_="form-control", value=value, diff --git a/shiny/ui/_input_password.py b/shiny/ui/_input_password.py index 8e7ff8446..5bbe041cd 100644 --- a/shiny/ui/_input_password.py +++ b/shiny/ui/_input_password.py @@ -5,6 +5,7 @@ from htmltools import tags, Tag, div, css, TagChildArg from .._docstring import add_example +from .._namespaces import namespaced_id from ._utils import shiny_input_label @@ -50,7 +51,7 @@ def input_password( return div( shiny_input_label(id, label), tags.input( - id=id, + id=namespaced_id(id), type="password", value=value, class_="form-control", diff --git a/shiny/ui/_input_select.py b/shiny/ui/_input_select.py index bcc63c627..31de8ddd0 100644 --- a/shiny/ui/_input_select.py +++ b/shiny/ui/_input_select.py @@ -9,6 +9,7 @@ from .._docstring import add_example from ._html_dependencies import selectize_deps +from .._namespaces import namespaced_id from ._utils import shiny_input_label _Choices = Dict[str, TagChildArg] @@ -166,7 +167,7 @@ def input_select( div( tags.select( *choices_tags, - id=id, + id=namespaced_id(id), class_=None if selectize else "form-select", multiple=multiple, width=width, diff --git a/shiny/ui/_input_slider.py b/shiny/ui/_input_slider.py index aca2a3ce5..b91f34811 100644 --- a/shiny/ui/_input_slider.py +++ b/shiny/ui/_input_slider.py @@ -14,6 +14,7 @@ from .._docstring import add_example from ._html_dependencies import ionrangeslider_deps +from .._namespaces import namespaced_id from ._utils import shiny_input_label # Even though TypedDict is available in Python 3.8, because it's used with NotRequired, @@ -169,6 +170,8 @@ def input_slider( scale_factor = math.ceil(n_steps / 10) n_ticks = n_steps / scale_factor + id = namespaced_id(id) + props: Dict[str, TagAttrArg] = { "class_": "js-range-slider", "id": id, diff --git a/shiny/ui/_input_text.py b/shiny/ui/_input_text.py index 05a25bf91..f1f2cd88a 100644 --- a/shiny/ui/_input_text.py +++ b/shiny/ui/_input_text.py @@ -11,6 +11,7 @@ from htmltools import Tag, TagChildArg, css, div, tags from .._docstring import add_example +from .._namespaces import namespaced_id from ._utils import shiny_input_label @@ -69,7 +70,7 @@ def input_text( return div( shiny_input_label(id, label), tags.input( - id=id, + id=namespaced_id(id), type="text", class_="form-control", value=value, @@ -157,7 +158,7 @@ def input_text_area( area = tags.textarea( value, - id=id, + id=namespaced_id(id), class_="form-control", style=css(width=None if width else "100%", height=height, resize=resize), placeholder=placeholder, diff --git a/shiny/ui/_input_update.py b/shiny/ui/_input_update.py index 725a8af0a..f241ea048 100644 --- a/shiny/ui/_input_update.py +++ b/shiny/ui/_input_update.py @@ -41,6 +41,7 @@ from ._input_date import _as_date_attr from ._input_select import SelectChoicesArg, _normalize_choices, _render_choices from ._input_slider import SliderValueArg, SliderStepArg, _slider_type, _as_numeric +from .._namespaces import namespaced_id from .._utils import drop_none from ..session import Session, require_active_session @@ -192,7 +193,7 @@ def update_checkbox_group( """ _update_choice_input( - id=id, + id=namespaced_id(id), type="checkbox", label=label, choices=choices, @@ -244,7 +245,7 @@ def update_radio_buttons( """ _update_choice_input( - id=id, + id=namespaced_id(id), type="radio", label=label, choices=choices, @@ -268,7 +269,11 @@ def _update_choice_input( options = None if choices is not None: opts = _generate_options( - id=id, type=type, choices=choices, selected=selected, inline=inline + id=namespaced_id(id), + type=type, + choices=choices, + selected=selected, + inline=inline, ) options = session._process_ui(opts)["html"] msg = {"label": label, "options": options, "value": selected} diff --git a/shiny/ui/_navs.py b/shiny/ui/_navs.py index 77abcb04b..399b411a6 100644 --- a/shiny/ui/_navs.py +++ b/shiny/ui/_navs.py @@ -27,6 +27,7 @@ from ._bootstrap import row, column from .._docstring import add_example from ._html_dependencies import bootstrap_deps +from .._namespaces import namespaced_id from ..types import NavSetArg from .._utils import private_random_int @@ -408,7 +409,7 @@ def navset_tab( return NavSet( *args, ul_class="nav nav-tabs", - id=id, + id=namespaced_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -460,7 +461,7 @@ def navset_pill( return NavSet( *args, ul_class="nav nav-pills", - id=id, + id=namespaced_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -510,7 +511,7 @@ def navset_hidden( return NavSet( *args, ul_class="nav nav-hidden", - id=id, + id=namespaced_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -590,7 +591,7 @@ def navset_tab_card( return NavSetCard( *args, ul_class="nav nav-tabs card-header-tabs", - id=id, + id=namespaced_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -646,7 +647,7 @@ def navset_pill_card( return NavSetCard( *args, ul_class="nav nav-pills card-header-pills", - id=id, + id=namespaced_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -740,7 +741,7 @@ def navset_pill_list( return NavSetPillList( *args, ul_class="nav nav-pills nav-stacked", - id=id, + id=namespaced_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -901,7 +902,7 @@ def navset_bar( return NavSetBar( *args, ul_class="nav navbar-nav", - id=id, + id=namespaced_id(id) if id else None, selected=selected, title=title, position=position, diff --git a/shiny/ui/_output.py b/shiny/ui/_output.py index 77d9844e0..5c633ed63 100644 --- a/shiny/ui/_output.py +++ b/shiny/ui/_output.py @@ -10,6 +10,7 @@ from htmltools import tags, Tag, div, css, TagAttrArg, TagFunction from .._docstring import add_example +from .._namespaces import namespaced_id @add_example() @@ -42,7 +43,7 @@ def output_plot( ~shiny.render.plot ~shiny.ui.output_image """ - res = output_image(id=id, width=width, height=height, inline=inline) + res = output_image(id=namespaced_id(id), width=width, height=height, inline=inline) res.add_class("shiny-plot-output") return res @@ -76,7 +77,7 @@ def output_image( """ func = tags.span if inline else div style = None if inline else css(width=width, height=height) - return func(id=id, class_="shiny-image-output", style=style) + return func(id=namespaced_id(id), class_="shiny-image-output", style=style) @add_example() @@ -111,7 +112,7 @@ def output_text( if not container: container = tags.span if inline else tags.div - return container(id=id, class_="shiny-text-output") + return container(id=namespaced_id(id), class_="shiny-text-output") def output_text_verbatim(id: str, placeholder: bool = False) -> Tag: @@ -145,7 +146,7 @@ def output_text_verbatim(id: str, placeholder: bool = False) -> Tag: """ cls = "shiny-text-output" + (" noplaceholder" if not placeholder else "") - return tags.pre(id=id, class_=cls) + return tags.pre(id=namespaced_id(id), class_=cls) @add_example() @@ -181,4 +182,4 @@ def output_ui( if not container: container = tags.span if inline else tags.div - return container({"class": "shiny-html-output"}, id=id, **kwargs) + return container({"class": "shiny-html-output"}, id=namespaced_id(id), **kwargs) diff --git a/shiny/ui/_page.py b/shiny/ui/_page.py index fb6a2a877..238532cad 100644 --- a/shiny/ui/_page.py +++ b/shiny/ui/_page.py @@ -28,6 +28,7 @@ from .._docstring import add_example from ._html_dependencies import bootstrap_deps from ._navs import navset_bar +from .._namespaces import namespaced_id from ..types import MISSING, MISSING_TYPE, NavSetArg from ._utils import get_window_title @@ -114,7 +115,7 @@ def page_navbar( navset_bar( *args, title=title, - id=id, + id=namespaced_id(id) if id else None, selected=selected, position=position, header=header, diff --git a/tests/test_modules.py b/tests/test_modules.py index ae8fac20e..d8681a430 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -1,6 +1,6 @@ """Tests for `Module`.""" -from typing import Callable, Dict, Union, cast +from typing import Dict, Union, cast import pytest from shiny import * @@ -11,10 +11,10 @@ from htmltools import TagChildArg -def mod_ui(ns: Callable[[str], str]) -> TagChildArg: +def mod_ui() -> TagChildArg: return ui.TagList( - ui.input_action_button(id=ns("button"), label="module1"), - ui.output_text_verbatim(id=ns("out")), + ui.input_action_button(id="button", label="module1"), + ui.output_text_verbatim(id="out"), ) @@ -38,8 +38,8 @@ def out() -> str: def test_module_ui(): x = cast(ui.TagList, mod.ui("mod1")) - assert cast(ui.Tag, x[0]).attrs["id"] == "mod1-button" - assert cast(ui.Tag, x[1]).attrs["id"] == "mod1-out" + assert cast(ui.Tag, x[0]).attrs["id"] == "mod1_button" + assert cast(ui.Tag, x[1]).attrs["id"] == "mod1_out" @pytest.mark.asyncio @@ -52,7 +52,7 @@ async def test_inputs_proxy(): # Different ways of accessing "a" from the input proxy. assert input_proxy.a.is_set() is False assert input_proxy["a"].is_set() is False - assert input["mod1-a"].is_set() is False + assert input["mod1_a"].is_set() is False input_proxy.a._set(2) @@ -60,7 +60,7 @@ async def test_inputs_proxy(): assert input.a() == 1 assert input_proxy.a() == 2 assert input_proxy["a"]() == 2 - assert input["mod1-a"]() == 2 + assert input["mod1_a"]() == 2 # Nested input proxies input_proxy_proxy = ModuleInputs("mod2", input_proxy) @@ -70,7 +70,7 @@ async def test_inputs_proxy(): # Different ways of accessing "a" from the input proxy. assert input_proxy_proxy.a.is_set() is False assert input_proxy_proxy["a"].is_set() is False - assert input_proxy["mod1-a"].is_set() is False + assert input_proxy["mod1_a"].is_set() is False input_proxy_proxy.a._set(3) @@ -79,7 +79,7 @@ async def test_inputs_proxy(): assert input_proxy.a() == 2 assert input_proxy_proxy.a() == 3 assert input_proxy_proxy["a"]() == 3 - assert input["mod1-mod2-a"]() == 3 + assert input["mod1_mod2_a"]() == 3 def test_current_session(): @@ -132,7 +132,7 @@ def _(): assert sessions["inner"] is sessions["inner_current"] assert sessions["inner_current"] is sessions["inner_calc_current"] assert isinstance(sessions["inner_current"], ModuleSession) - assert sessions["inner_current"]._ns == "mod_inner" + assert sessions["inner_current"]._ns == "mod_outer_mod_inner" assert sessions["outer"] is sessions["outer_current"] assert sessions["outer_current"] is sessions["outer_calc_current"] From 64f05bfde2a21c94ca75d69eaf1a6ab1c9a94092 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 29 Apr 2022 10:56:14 -0500 Subject: [PATCH 02/13] Experiment with a decorator based API --- docs/source/index.rst | 3 +- examples/moduleapp/app.py | 16 +++-- shiny/__init__.py | 5 +- shiny/_modules.py | 137 ++++++++++++++++++++++---------------- 4 files changed, 93 insertions(+), 68 deletions(-) diff --git a/docs/source/index.rst b/docs/source/index.rst index 412530f44..f5ca5a27a 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -303,7 +303,8 @@ Control application complexity by namespacing UI and server code. :toctree: reference/ :template: class.rst - Module + module_ui + module_server Type hints diff --git a/examples/moduleapp/app.py b/examples/moduleapp/app.py index cd785247f..8ef6d3ddb 100644 --- a/examples/moduleapp/app.py +++ b/examples/moduleapp/app.py @@ -3,6 +3,7 @@ # ============================================================ # Counter module # ============================================================ +@module_ui def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: return ui.div( {"style": "border: 1px solid #ccc; border-radius: 5px; margin: 5px 0;"}, @@ -12,7 +13,8 @@ def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: ) -def counter_server(input: Inputs, output: Outputs, session: Session): +@module_server +def counter_server(input: Inputs, output: Outputs, session: Session) -> int: count: reactive.Value[int] = reactive.Value(0) @reactive.Effect @@ -25,22 +27,22 @@ def _(): def out() -> str: return f"Click count is {count()}" - -counter_module = Module(counter_ui, counter_server) + return 1 # ============================================================================= # App that uses module # ============================================================================= app_ui = ui.page_fluid( - counter_module.ui("counter1", "Counter 1"), - counter_module.ui("counter2", "Counter 2"), + counter_ui("counter1", "Counter 1"), + counter_ui("counter2", "Counter 2"), ) def server(input: Inputs, output: Outputs, session: Session): - counter_module.server("counter1") - counter_module.server("counter2") + counter_server("counter1") + val = counter_server("counter2") + print(val) app = App(app_ui, server) diff --git a/shiny/__init__.py b/shiny/__init__.py index c846f5bdb..597a01f43 100644 --- a/shiny/__init__.py +++ b/shiny/__init__.py @@ -13,7 +13,7 @@ # Private submodules that have some user-facing functionality from ._app import App from ._decorators import event -from ._modules import Module +from ._modules import module_ui, module_server from ._validation import req from ._deprecated import * @@ -43,7 +43,8 @@ # _main.py "run_app", # _modules.py - "Module", + "module_ui", + "module_server", # _session.py "Session", "Inputs", diff --git a/shiny/_modules.py b/shiny/_modules.py index 638ddcf0a..a65802dea 100644 --- a/shiny/_modules.py +++ b/shiny/_modules.py @@ -1,10 +1,7 @@ -__all__ = ("Module", "namespaced_id") +__all__ = ("namespaced_id", "module_ui", "module_server") -from typing import Any, Callable, Optional +from typing import Any, Callable, TypeVar, ParamSpec, Concatenate -from htmltools import TagChildArg - -from ._docstring import add_example from ._namespaces import namespaced_id from .session import Inputs, Outputs, Session, require_active_session, session_context @@ -78,59 +75,83 @@ def __init__(self, ns: str): self._ns = ns -@add_example() -class Module: - """ - Modularize UI and server-side logic. - - Parameters - ---------- - ui - The module's UI definition. - server - The module's server-side logic. - """ +P = ParamSpec("P") +R = TypeVar("R") + - def __init__( - self, - ui: Callable[..., TagChildArg], - server: Callable[[ModuleInputs, ModuleOutputs, ModuleSession], None], - ) -> None: - self._ui: Callable[..., TagChildArg] = ui - self._server: Callable[ - [ModuleInputs, ModuleOutputs, ModuleSession], None - ] = server - - def ui(self, ns: str, *args: Any) -> TagChildArg: - """ - Render the module's UI. - - Parameters - ---------- - namespace - A namespace for the module. - args - Additional arguments to pass to the module's UI definition. - """ - - # Create a fake session so that namespaced_id() knows - # what the relevant namespace is +def module_ui(fn: Callable[P, R]) -> Callable[Concatenate[str, P], R]: + def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: with session_context(MockModuleSession(ns)): - return self._ui(*args) - - def server(self, ns: str, *, session: Optional[Session] = None) -> None: - """ - Invoke the module's server-side logic. - - Parameters - ---------- - ns - A namespace for the module. - session - A :class:`~shiny.Session` instance. If not provided, it is inferred via - :func:`~shiny.session.get_current_session`. - """ - - mod_sess = ModuleSession(ns, require_active_session(session)) + return fn(*args, **kwargs) + + return wrapper + + +def module_server( + fn: Callable[Concatenate[Inputs, Outputs, Session, P], R] +) -> Callable[Concatenate[str, P], R]: + def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: + mod_sess = ModuleSession(ns, require_active_session(None)) with session_context(mod_sess): - return self._server(mod_sess.input, mod_sess.output, mod_sess) + return fn(mod_sess.input, mod_sess.output, mod_sess, *args, **kwargs) + + return wrapper + + +# @add_example() +# class Module: +# """ +# Modularize UI and server-side logic. +# +# Parameters +# ---------- +# ui +# The module's UI definition. +# server +# The module's server-side logic. +# """ +# +# def __init__( +# self, +# ui: Callable[..., TagChildArg], +# server: Callable[[ModuleInputs, ModuleOutputs, ModuleSession], None], +# ) -> None: +# self._ui: Callable[..., TagChildArg] = ui +# self._server: Callable[ +# [ModuleInputs, ModuleOutputs, ModuleSession], None +# ] = server +# +# def ui(self, ns: str, *args: Any) -> TagChildArg: +# """ +# Render the module's UI. +# +# Parameters +# ---------- +# namespace +# A namespace for the module. +# args +# Additional arguments to pass to the module's UI definition. +# """ +# +# # Create a fake session so that namespaced_id() knows +# # what the relevant namespace is +# with session_context(MockModuleSession(ns)): +# return self._ui(*args) +# +# def server(self, ns: str, *, session: Optional[Session] = None) -> None: +# """ +# Invoke the module's server-side logic. +# +# Parameters +# ---------- +# ns +# A namespace for the module. +# session +# A :class:`~shiny.Session` instance. If not provided, it is inferred via +# :func:`~shiny.session.get_current_session`. +# """ +# +# mod_sess = ModuleSession(ns, require_active_session(session)) +# with session_context(mod_sess): +# return self._server(mod_sess.input, mod_sess.output, mod_sess) +# From 2cd395f50c1c38be7570f6da7d18412ece44ce99 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 20 May 2022 17:10:19 -0500 Subject: [PATCH 03/13] Move away from ModuleSession/ModuleInput/ModuleOutput toward SessionProxy --- shiny/_modules.py | 149 ++++---------------------------------- shiny/_namespaces.py | 51 +++++++------ shiny/session/_session.py | 94 ++++++++++++++++++------ tests/test_modules.py | 116 ++++++++++------------------- 4 files changed, 153 insertions(+), 257 deletions(-) diff --git a/shiny/_modules.py b/shiny/_modules.py index a65802dea..8d81672fe 100644 --- a/shiny/_modules.py +++ b/shiny/_modules.py @@ -1,79 +1,15 @@ __all__ = ("namespaced_id", "module_ui", "module_server") -from typing import Any, Callable, TypeVar, ParamSpec, Concatenate +import sys +from typing import Callable, TypeVar -from ._namespaces import namespaced_id -from .session import Inputs, Outputs, Session, require_active_session, session_context - - -class ModuleInputs(Inputs): - """ - A class representing a module's outputs. - - Warning - ------- - An instance of this class is created for each request and passed as an argument to - the :class:`shiny.modules.Module`'s ``server`` function. For this reason, you - shouldn't need to create instances of this class yourself. Furthermore, you - probably shouldn't need this class for type checking either since it has the same - signature as :class:`shiny.session.Session`. - """ - - def __init__(self, ns: str, parent_inputs: Inputs): - self._ns = namespaced_id(ns, parent_inputs._ns) # Support nested modules - # Don't set _parent attribute like the other classes since Inputs redefines - # __setattr__ - self._map = parent_inputs._map - - -class ModuleOutputs(Outputs): - """ - A class representing a module's outputs. - - Warning - ------- - An instance of this class is created for each request and passed as an argument to - the :class:`shiny.modules.Module`'s ``server`` function. For this reason, you - shouldn't need to create instances of this class yourself. Furthermore, you - probably shouldn't need this class for type checking either since it has the same - signature as :class:`shiny.session.Session`. - """ - - def __init__(self, ns: str, parent_outputs: Outputs): - self._ns = namespaced_id(ns, parent_outputs._ns) # Support nested modules - self._parent = parent_outputs - - def __getattr__(self, attr: str) -> Any: - return getattr(self._parent, attr) - - -class ModuleSession(Session): - """ - A class representing a module's outputs. - - Warning - ------- - An instance of this class is created for each request and passed as an argument to - the :class:`shiny.modules.Module`'s ``server`` function. For this reason, you - shouldn't need to create instances of this class yourself. Furthermore, you - probably shouldn't need this class for type checking either since it has the same - signature as :class:`shiny.session.Session`. - """ - - def __init__(self, ns: str, parent_session: Session): - self._ns: str = namespaced_id(ns, parent_session._ns) # Support nested modules - self._parent: Session = parent_session - self.input: ModuleInputs = ModuleInputs(ns, parent_session.input) - self.output: ModuleOutputs = ModuleOutputs(ns, parent_session.output) - - def __getattr__(self, attr: str) -> Any: - return getattr(self._parent, attr) - - -class MockModuleSession(ModuleSession): - def __init__(self, ns: str): - self._ns = ns +if sys.version_info < (3, 10): + from typing_extensions import ParamSpec, Concatenate +else: + from typing import ParamSpec, Concatenate +from ._namespaces import namespaced_id, namespace_context, get_current_namespaces +from .session import Inputs, Outputs, Session, require_active_session, session_context P = ParamSpec("P") R = TypeVar("R") @@ -81,7 +17,8 @@ def __init__(self, ns: str): def module_ui(fn: Callable[P, R]) -> Callable[Concatenate[str, P], R]: def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: - with session_context(MockModuleSession(ns)): + # TODO: what should happen if this is called *inside* of a session? Do we tack on the parent session's namespace as well? + with namespace_context(get_current_namespaces() + [ns]): return fn(*args, **kwargs) return wrapper @@ -91,67 +28,9 @@ def module_server( fn: Callable[Concatenate[Inputs, Outputs, Session, P], R] ) -> Callable[Concatenate[str, P], R]: def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: - mod_sess = ModuleSession(ns, require_active_session(None)) - with session_context(mod_sess): - return fn(mod_sess.input, mod_sess.output, mod_sess, *args, **kwargs) + sess = require_active_session(None) + child_sess = sess.make_scope(ns) + with session_context(child_sess): + return fn(child_sess.input, child_sess.output, child_sess, *args, **kwargs) return wrapper - - -# @add_example() -# class Module: -# """ -# Modularize UI and server-side logic. -# -# Parameters -# ---------- -# ui -# The module's UI definition. -# server -# The module's server-side logic. -# """ -# -# def __init__( -# self, -# ui: Callable[..., TagChildArg], -# server: Callable[[ModuleInputs, ModuleOutputs, ModuleSession], None], -# ) -> None: -# self._ui: Callable[..., TagChildArg] = ui -# self._server: Callable[ -# [ModuleInputs, ModuleOutputs, ModuleSession], None -# ] = server -# -# def ui(self, ns: str, *args: Any) -> TagChildArg: -# """ -# Render the module's UI. -# -# Parameters -# ---------- -# namespace -# A namespace for the module. -# args -# Additional arguments to pass to the module's UI definition. -# """ -# -# # Create a fake session so that namespaced_id() knows -# # what the relevant namespace is -# with session_context(MockModuleSession(ns)): -# return self._ui(*args) -# -# def server(self, ns: str, *, session: Optional[Session] = None) -> None: -# """ -# Invoke the module's server-side logic. -# -# Parameters -# ---------- -# ns -# A namespace for the module. -# session -# A :class:`~shiny.Session` instance. If not provided, it is inferred via -# :func:`~shiny.session.get_current_session`. -# """ -# -# mod_sess = ModuleSession(ns, require_active_session(session)) -# with session_context(mod_sess): -# return self._server(mod_sess.input, mod_sess.output, mod_sess) -# diff --git a/shiny/_namespaces.py b/shiny/_namespaces.py index c6c876167..418a19150 100644 --- a/shiny/_namespaces.py +++ b/shiny/_namespaces.py @@ -1,34 +1,39 @@ -# TODO: make this available under the shiny.modules API -__all__ = ("namespaced_id",) +from contextlib import contextmanager +from contextvars import ContextVar, Token +from typing import Union, List -from typing import Union, Optional -from .types import MISSING, MISSING_TYPE +class ResolvedId(str): + pass -def namespaced_id(id: str, ns: Union[str, MISSING_TYPE, None] = MISSING) -> str: - """ - Namespace an ID based on the current ``Module()``'s namespace. +Id = Union[str, ResolvedId] - Parameters - ---------- - id - The ID to namespace.. - """ - if isinstance(ns, MISSING_TYPE): - ns = get_current_namespace() - if ns is None: +def namespaced_id(id: Id) -> Id: + return namespaced_id_ns(id, get_current_namespaces()) + + +def namespaced_id_ns(id: Id, namespaces: List[str] = []) -> Id: + if isinstance(id, ResolvedId) or len(namespaces) == 0: return id else: - return ns + "_" + id + return ResolvedId("_".join(namespaces) + "_" + id) -def get_current_namespace() -> Optional[str]: - from .session import get_current_session +def get_current_namespaces() -> List[str]: + return _current_namespaces.get() - session = get_current_session() - if session is None: - return None - else: - return session._ns + +_current_namespaces: ContextVar[List[str]] = ContextVar( + "current_namespaces", default=[] +) + + +@contextmanager +def namespace_context(namespaces: List[str]): + token: Token[List[str]] = _current_namespaces.set(namespaces) + try: + yield + finally: + _current_namespaces.reset(token) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index 913f00615..b5236791f 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -49,7 +49,7 @@ from .._fileupload import FileInfo, FileUploadManager from ..http_staticfiles import FileResponse from ..input_handler import input_handlers -from .._namespaces import namespaced_id +from .._namespaces import namespaced_id_ns from ..reactive import Value, Effect, Effect_, isolate, flush from ..reactive._core import lock from ..types import SafeException, SilentCancelOutputException, SilentException @@ -112,7 +112,14 @@ def empty_outbound_message_queues() -> OutBoundMessageQueues: return {"values": [], "input_messages": [], "errors": []} -class Session: +# Makes isinstance(x, Session) also return True when x is a SessionProxy (i.e., a module +# session) +class SessionMeta(type): + def __instancecheck__(self, __instance: Any) -> bool: + return isinstance(__instance, SessionProxy) + + +class Session(object, metaclass=SessionMeta): """ A class representing a user session. @@ -142,8 +149,6 @@ def __init__( self.input: Inputs = Inputs() self.output: Outputs = Outputs(self) - self._ns: Optional[str] = None # Only relevant for ModuleSession - self.user: Union[str, None] = None self.groups: Union[List[str], None] = None credentials_json: str = "" @@ -475,7 +480,7 @@ def send_input_message(self, id: str, message: Dict[str, object]) -> None: message The message to send. """ - msg: Dict[str, object] = {"id": namespaced_id(id, self._ns), "message": message} + msg: Dict[str, object] = {"id": id, "message": message} self._outbound_message_queues["input_messages"].append(msg) self._request_flush() @@ -643,7 +648,7 @@ async def _unhandled_error(self, e: Exception) -> None: await self.close() # TODO: probably name should be id - # TODO: anything to be done here for module support? + @add_example() def download( self, @@ -729,6 +734,46 @@ def _process_ui(self, ui: TagChildArg) -> RenderedDeps: return {"deps": deps, "html": res["html"]} + @staticmethod + def ns(id: str) -> str: + return id + + def make_scope(self, id: str) -> "Session": + ns = create_ns_func(id) + return SessionProxy(parent=self, ns=ns) # type: ignore + + +class SessionProxy: + def __init__(self, parent: Session, ns: Callable[[str], str]) -> None: + self._parent = parent + self.ns = ns + self.input = Inputs(values=parent.input._map, ns=ns) + self.output = Outputs( + session=cast(Session, self), + effects=self.output._effects, + suspend_when_hidden=self.output._suspend_when_hidden, + ns=ns, + ) + + def __getattr__(self, attr: str) -> Any: + return getattr(self._parent, attr) + + def send_input_message(self, id: str, message: Dict[str, object]) -> None: + return self._parent.send_input_message(self.ns(id), message) + + # TODO: test this actually works (will handle_request() need an override?) + def download( + self, name: str, **kwargs: object + ) -> Callable[[DownloadHandler], None]: + return self._parent.download(self.ns(name), **kwargs) + + def make_scope(self, id: str) -> Session: + return self._parent.make_scope(self.ns(id)) + + +def create_ns_func(namespace: str) -> Callable[[str], str]: + return lambda x: namespaced_id_ns(x, [namespace]) + # ====================================================================================== # Inputs @@ -748,31 +793,30 @@ class Inputs: for type checking reasons). """ - def __init__(self, **kwargs: object) -> None: - self._map: dict[str, Value[Any]] = {} - for key, value in kwargs.items(): - self._map[key] = Value(value, read_only=True) - - self._ns: Optional[str] = None # Only relevant for ModuleInputs() + def __init__( + self, values: Dict[str, Value[Any]] = {}, ns: Callable[[str], str] = lambda x: x + ) -> None: + self._map = values + self._ns = ns def __setitem__(self, key: str, value: Value[Any]) -> None: if not isinstance(value, Value): raise TypeError("`value` must be a reactive.Value object.") - self._map[namespaced_id(key, self._ns)] = value + self._map[self._ns(key)] = value def __getitem__(self, key: str) -> Value[Any]: - key = namespaced_id(key, self._ns) + key = self._ns(key) # Auto-populate key if accessed but not yet set. Needed to take reactive # dependencies on input values that haven't been received from client # yet. if key not in self._map: - self._map[key] = Value(read_only=True) + self._map[key] = Value[Any](read_only=True) return self._map[key] def __delitem__(self, key: str) -> None: - del self._map[namespaced_id(key, self._ns)] + del self._map[self._ns(key)] # Allow access of values as attributes. def __setattr__(self, attr: str, value: Value[Any]) -> None: @@ -806,11 +850,17 @@ class Outputs: for type checking reasons). """ - def __init__(self, session: Session) -> None: - self._effects: Dict[str, Effect_] = {} - self._suspend_when_hidden: Dict[str, bool] = {} - self._session: Session = session - self._ns: Optional[str] = None # Only relevant for ModuleOutputs() + def __init__( + self, + session: Session, + ns: Callable[[str], str] = lambda x: x, + effects: Dict[str, Effect_] = {}, + suspend_when_hidden: Dict[str, bool] = {}, + ) -> None: + self._session = session + self._ns = ns + self._effects = effects + self._suspend_when_hidden = suspend_when_hidden @overload def __call__(self, fn: render.RenderFunction) -> None: @@ -846,7 +896,7 @@ def __call__( def set_fn(fn: render.RenderFunction) -> None: # Get the (possibly namespaced) output id - output_name = namespaced_id(id or fn.__name__, self._ns) + output_name = self._ns(id or fn.__name__) # fn is either a regular function or a RenderFunction object. If # it's the latter, give it a bit of metadata. diff --git a/tests/test_modules.py b/tests/test_modules.py index d8681a430..e5af66454 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -2,90 +2,46 @@ from typing import Dict, Union, cast -import pytest from shiny import * from shiny.session import get_current_session from shiny._connection import MockConnection -from shiny._modules import ModuleInputs, ModuleSession +from shiny._namespaces import namespaced_id from shiny._utils import run_coro_sync -from htmltools import TagChildArg +from htmltools import TagList, Tag -def mod_ui() -> TagChildArg: - return ui.TagList( - ui.input_action_button(id="button", label="module1"), - ui.output_text_verbatim(id="out"), +@module_ui +def mod_inner() -> TagList: + return TagList( + ui.input_action_button("button", label="inner"), + ui.output_text(namespaced_id("out")), ) -# Note: We currently can't test Session; this is just here for future use. -def mod_server(input: Inputs, output: Outputs, session: Session): - count: reactive.Value[int] = reactive.Value(0) +@module_ui +def mod_outer() -> TagList: + return TagList(mod_inner("inner"), ui.output_text("out2")) - @reactive.Effect - @event(session.input.button) - def _(): - count.set(count() + 1) - @output - @render.text - def out() -> str: - return f"Click count is {count()}" - - -mod = Module(mod_ui, mod_server) +def get_id(x: TagList, child_idx: int = 0) -> str: + return cast(Tag, x[child_idx]).attrs["id"] def test_module_ui(): - x = cast(ui.TagList, mod.ui("mod1")) - assert cast(ui.Tag, x[0]).attrs["id"] == "mod1_button" - assert cast(ui.Tag, x[1]).attrs["id"] == "mod1_out" - - -@pytest.mark.asyncio -async def test_inputs_proxy(): - input = Inputs(a=1) - input_proxy = ModuleInputs("mod1", input) - - with reactive.isolate(): - assert input.a() == 1 - # Different ways of accessing "a" from the input proxy. - assert input_proxy.a.is_set() is False - assert input_proxy["a"].is_set() is False - assert input["mod1_a"].is_set() is False - - input_proxy.a._set(2) + x = mod_inner("inner") + assert get_id(x, 0) == "inner_button" + assert get_id(x, 1) == "inner_out" + y = mod_outer("outer") + assert get_id(y, 0) == "outer_inner_button" + assert get_id(y, 1) == "outer_inner_out" + assert get_id(y, 2) == "outer_out2" - with reactive.isolate(): - assert input.a() == 1 - assert input_proxy.a() == 2 - assert input_proxy["a"]() == 2 - assert input["mod1_a"]() == 2 - # Nested input proxies - input_proxy_proxy = ModuleInputs("mod2", input_proxy) - with reactive.isolate(): - assert input.a() == 1 - assert input_proxy.a() == 2 - # Different ways of accessing "a" from the input proxy. - assert input_proxy_proxy.a.is_set() is False - assert input_proxy_proxy["a"].is_set() is False - assert input_proxy["mod1_a"].is_set() is False +def test_session_scoping(): - input_proxy_proxy.a._set(3) - - with reactive.isolate(): - assert input.a() == 1 - assert input_proxy.a() == 2 - assert input_proxy_proxy.a() == 3 - assert input_proxy_proxy["a"]() == 3 - assert input["mod1_mod2_a"]() == 3 - - -def test_current_session(): - - sessions: Dict[str, Union[Session, None]] = {} + sessions: Dict[str, Union[Session, None, str]] = {} + @module_server def inner(input: Inputs, output: Outputs, session: Session): @reactive.Calc def out(): @@ -96,9 +52,10 @@ def _(): sessions["inner"] = session sessions["inner_current"] = get_current_session() sessions["inner_calc_current"] = out() + sessions["inner_id"] = session.ns("foo") + sessions["inner_ui_id"] = get_id(mod_outer("outer"), 0) - mod_inner = Module(ui.TagList, inner) - + @module_server def outer(input: Inputs, output: Outputs, session: Session): @reactive.Calc def out(): @@ -106,15 +63,15 @@ def out(): @reactive.Effect def _(): - mod_inner.server("mod_inner") + inner("mod_inner") sessions["outer"] = session sessions["outer_current"] = get_current_session() sessions["outer_calc_current"] = out() - - mod_outer = Module(ui.TagList, outer) + sessions["outer_id"] = session.ns("foo") + sessions["outer_ui_id"] = get_id(mod_outer("outer"), 0) def server(input: Inputs, output: Outputs, session: Session): - mod_outer.server("mod_outer") + outer("mod_outer") @reactive.Calc def out(): @@ -125,21 +82,26 @@ def _(): sessions["top"] = session sessions["top_current"] = get_current_session() sessions["top_calc_current"] = out() + sessions["top_id"] = session.ns("foo") + sessions["top_ui_id"] = get_id(mod_outer("outer"), 0) App(ui.TagList(), server)._create_session(MockConnection()) run_coro_sync(reactive.flush()) assert sessions["inner"] is sessions["inner_current"] assert sessions["inner_current"] is sessions["inner_calc_current"] - assert isinstance(sessions["inner_current"], ModuleSession) - assert sessions["inner_current"]._ns == "mod_outer_mod_inner" + assert isinstance(sessions["inner_current"], Session) + assert sessions["inner_id"] == "mod_outer_mod_inner_foo" + assert sessions["inner_ui_id"] == "outer_inner_button" assert sessions["outer"] is sessions["outer_current"] assert sessions["outer_current"] is sessions["outer_calc_current"] - assert isinstance(sessions["outer_current"], ModuleSession) - assert sessions["outer_current"]._ns == "mod_outer" + assert isinstance(sessions["outer_current"], Session) + assert sessions["outer_id"] == "mod_outer_foo" + assert sessions["outer_ui_id"] == "outer_inner_button" assert sessions["top"] is sessions["top_current"] assert sessions["top_current"] is sessions["top_calc_current"] assert isinstance(sessions["top_current"], Session) - assert not isinstance(sessions["top_current"], ModuleSession) + assert sessions["top_id"] == "foo" + assert sessions["top_ui_id"] == "outer_inner_button" From 898e534ba0f68bd0c1afd318e0ef0b74c9e1c562 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 17 Jun 2022 11:56:09 -0500 Subject: [PATCH 04/13] Fix examples --- examples/moduleapp/app.py | 11 +++++------ shiny/examples/Module/app.py | 19 ++++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/examples/moduleapp/app.py b/examples/moduleapp/app.py index 8ef6d3ddb..e90b15a3e 100644 --- a/examples/moduleapp/app.py +++ b/examples/moduleapp/app.py @@ -14,8 +14,10 @@ def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: @module_server -def counter_server(input: Inputs, output: Outputs, session: Session) -> int: - count: reactive.Value[int] = reactive.Value(0) +def counter_server( + input: Inputs, output: Outputs, session: Session, starting_value: int = 0 +): + count: reactive.Value[int] = reactive.Value(starting_value) @reactive.Effect @event(input.button) @@ -27,8 +29,6 @@ def _(): def out() -> str: return f"Click count is {count()}" - return 1 - # ============================================================================= # App that uses module @@ -41,8 +41,7 @@ def out() -> str: def server(input: Inputs, output: Outputs, session: Session): counter_server("counter1") - val = counter_server("counter2") - print(val) + counter_server("counter2") app = App(app_ui, server) diff --git a/shiny/examples/Module/app.py b/shiny/examples/Module/app.py index cd785247f..e90b15a3e 100644 --- a/shiny/examples/Module/app.py +++ b/shiny/examples/Module/app.py @@ -3,6 +3,7 @@ # ============================================================ # Counter module # ============================================================ +@module_ui def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: return ui.div( {"style": "border: 1px solid #ccc; border-radius: 5px; margin: 5px 0;"}, @@ -12,8 +13,11 @@ def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: ) -def counter_server(input: Inputs, output: Outputs, session: Session): - count: reactive.Value[int] = reactive.Value(0) +@module_server +def counter_server( + input: Inputs, output: Outputs, session: Session, starting_value: int = 0 +): + count: reactive.Value[int] = reactive.Value(starting_value) @reactive.Effect @event(input.button) @@ -26,21 +30,18 @@ def out() -> str: return f"Click count is {count()}" -counter_module = Module(counter_ui, counter_server) - - # ============================================================================= # App that uses module # ============================================================================= app_ui = ui.page_fluid( - counter_module.ui("counter1", "Counter 1"), - counter_module.ui("counter2", "Counter 2"), + counter_ui("counter1", "Counter 1"), + counter_ui("counter2", "Counter 2"), ) def server(input: Inputs, output: Outputs, session: Session): - counter_module.server("counter1") - counter_module.server("counter2") + counter_server("counter1") + counter_server("counter2") app = App(app_ui, server) From 58327317f377459235b4e5aab40ade8319810ac9 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 17 Jun 2022 11:56:30 -0500 Subject: [PATCH 05/13] Fix downloads --- shiny/_modules.py | 1 - shiny/session/_session.py | 11 ++++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/shiny/_modules.py b/shiny/_modules.py index 8d81672fe..ccfa352e3 100644 --- a/shiny/_modules.py +++ b/shiny/_modules.py @@ -17,7 +17,6 @@ def module_ui(fn: Callable[P, R]) -> Callable[Concatenate[str, P], R]: def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: - # TODO: what should happen if this is called *inside* of a session? Do we tack on the parent session's namespace as well? with namespace_context(get_current_namespaces() + [ns]): return fn(*args, **kwargs) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index b5236791f..37ae4d5e7 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -647,8 +647,6 @@ async def _unhandled_error(self, e: Exception) -> None: print("Unhandled error: " + str(e)) await self.close() - # TODO: probably name should be id - @add_example() def download( self, @@ -761,11 +759,14 @@ def __getattr__(self, attr: str) -> Any: def send_input_message(self, id: str, message: Dict[str, object]) -> None: return self._parent.send_input_message(self.ns(id), message) - # TODO: test this actually works (will handle_request() need an override?) def download( - self, name: str, **kwargs: object + self, id: Optional[str] = None, **kwargs: object ) -> Callable[[DownloadHandler], None]: - return self._parent.download(self.ns(name), **kwargs) + def wrapper(fn: DownloadHandler): + id_ = self.ns(id or fn.__name__) + return self._parent.download(id=id_, **kwargs)(fn) + + return wrapper def make_scope(self, id: str) -> Session: return self._parent.make_scope(self.ns(id)) From 0ff664e1586f7a4813c87a538523cda61ebf8816 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 17 Jun 2022 12:12:55 -0500 Subject: [PATCH 06/13] Add dynamic_route method to SessionProxy --- shiny/session/_session.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index 37ae4d5e7..062c8a853 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -344,7 +344,6 @@ async def uploadEnd(job_id: str, input_id: str) -> None: # ========================================================================== # Handling /session/{session_id}/{action}/{subpath} requests # ========================================================================== - # TODO: anything to be done here for module support? async def _handle_request( self, request: Request, action: str, subpath: Optional[str] ) -> ASGIApp: @@ -756,9 +755,15 @@ def __init__(self, parent: Session, ns: Callable[[str], str]) -> None: def __getattr__(self, attr: str) -> Any: return getattr(self._parent, attr) + def make_scope(self, id: str) -> Session: + return self._parent.make_scope(self.ns(id)) + def send_input_message(self, id: str, message: Dict[str, object]) -> None: return self._parent.send_input_message(self.ns(id), message) + def dynamic_route(self, name: str, handler: DynamicRouteHandler) -> str: + return self._parent.dynamic_route(self.ns(name), handler) + def download( self, id: Optional[str] = None, **kwargs: object ) -> Callable[[DownloadHandler], None]: @@ -768,9 +773,6 @@ def wrapper(fn: DownloadHandler): return wrapper - def make_scope(self, id: str) -> Session: - return self._parent.make_scope(self.ns(id)) - def create_ns_func(namespace: str) -> Callable[[str], str]: return lambda x: namespaced_id_ns(x, [namespace]) From d50d6d997ac91c282cb2ec7c2ddc33095b8ab3b3 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 17 Jun 2022 17:27:26 -0500 Subject: [PATCH 07/13] Give ResolvedId a __call__ method --- pyrightconfig.json | 2 +- shiny/_modules.py | 10 +++++----- shiny/_namespaces.py | 35 ++++++++++++++++++----------------- shiny/session/_session.py | 18 ++++++------------ shiny/session/_utils.py | 4 +++- tests/test_modules.py | 28 ++++++++++++++-------------- tests/test_poll.py | 3 +++ 7 files changed, 50 insertions(+), 50 deletions(-) diff --git a/pyrightconfig.json b/pyrightconfig.json index 8d5ddb884..75e2b64bd 100644 --- a/pyrightconfig.json +++ b/pyrightconfig.json @@ -1,5 +1,5 @@ { - "ignore": ["shiny/examples", "examples", "build", "dist", "typings"], + "ignore": ["shiny/examples", "examples", "build", "dist", "typings", "sandbox"], "typeCheckingMode": "strict", "reportImportCycles": "none", "reportUnusedFunction": "none", diff --git a/shiny/_modules.py b/shiny/_modules.py index ccfa352e3..fe2693efe 100644 --- a/shiny/_modules.py +++ b/shiny/_modules.py @@ -8,7 +8,7 @@ else: from typing import ParamSpec, Concatenate -from ._namespaces import namespaced_id, namespace_context, get_current_namespaces +from ._namespaces import namespaced_id, namespace_context, Id from .session import Inputs, Outputs, Session, require_active_session, session_context P = ParamSpec("P") @@ -16,8 +16,8 @@ def module_ui(fn: Callable[P, R]) -> Callable[Concatenate[str, P], R]: - def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: - with namespace_context(get_current_namespaces() + [ns]): + def wrapper(id: Id, *args: P.args, **kwargs: P.kwargs) -> R: + with namespace_context(id): return fn(*args, **kwargs) return wrapper @@ -26,9 +26,9 @@ def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: def module_server( fn: Callable[Concatenate[Inputs, Outputs, Session, P], R] ) -> Callable[Concatenate[str, P], R]: - def wrapper(ns: str, *args: P.args, **kwargs: P.kwargs) -> R: + def wrapper(id: Id, *args: P.args, **kwargs: P.kwargs) -> R: sess = require_active_session(None) - child_sess = sess.make_scope(ns) + child_sess = sess.make_scope(id) with session_context(child_sess): return fn(child_sess.input, child_sess.output, child_sess, *args, **kwargs) diff --git a/shiny/_namespaces.py b/shiny/_namespaces.py index 418a19150..5525f9027 100644 --- a/shiny/_namespaces.py +++ b/shiny/_namespaces.py @@ -1,39 +1,40 @@ from contextlib import contextmanager from contextvars import ContextVar, Token -from typing import Union, List +from typing import Union class ResolvedId(str): - pass + def __call__(self, id: "Id") -> "ResolvedId": + if self == "" or isinstance(id, ResolvedId): + return ResolvedId(id) + else: + return ResolvedId(self + "_" + id) -Id = Union[str, ResolvedId] +Root = ResolvedId("") -def namespaced_id(id: Id) -> Id: - return namespaced_id_ns(id, get_current_namespaces()) +Id = Union[str, ResolvedId] -def namespaced_id_ns(id: Id, namespaces: List[str] = []) -> Id: - if isinstance(id, ResolvedId) or len(namespaces) == 0: - return id - else: - return ResolvedId("_".join(namespaces) + "_" + id) +def namespaced_id(id: Id) -> ResolvedId: + return ResolvedId(get_current_namespace())(id) -def get_current_namespaces() -> List[str]: - return _current_namespaces.get() +def get_current_namespace() -> ResolvedId: + return _current_namespace.get() -_current_namespaces: ContextVar[List[str]] = ContextVar( - "current_namespaces", default=[] +_current_namespace: ContextVar[ResolvedId] = ContextVar( + "current_namespace", default=Root ) @contextmanager -def namespace_context(namespaces: List[str]): - token: Token[List[str]] = _current_namespaces.set(namespaces) +def namespace_context(id: Union[Id, None]): + namespace = namespaced_id(id) if id else Root + token: Token[ResolvedId] = _current_namespace.set(namespace) try: yield finally: - _current_namespaces.reset(token) + _current_namespace.reset(token) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index 062c8a853..ad0591049 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -49,7 +49,7 @@ from .._fileupload import FileInfo, FileUploadManager from ..http_staticfiles import FileResponse from ..input_handler import input_handlers -from .._namespaces import namespaced_id_ns +from .._namespaces import ResolvedId, Id, Root from ..reactive import Value, Effect, Effect_, isolate, flush from ..reactive._core import lock from ..types import SafeException, SilentCancelOutputException, SilentException @@ -131,6 +131,8 @@ class Session(object, metaclass=SessionMeta): for type checking reasons). """ + ns = Root + # ========================================================================== # Initialization # ========================================================================== @@ -731,17 +733,13 @@ def _process_ui(self, ui: TagChildArg) -> RenderedDeps: return {"deps": deps, "html": res["html"]} - @staticmethod - def ns(id: str) -> str: - return id - - def make_scope(self, id: str) -> "Session": - ns = create_ns_func(id) + def make_scope(self, id: Id) -> "Session": + ns = self.ns(id) return SessionProxy(parent=self, ns=ns) # type: ignore class SessionProxy: - def __init__(self, parent: Session, ns: Callable[[str], str]) -> None: + def __init__(self, parent: Session, ns: ResolvedId) -> None: self._parent = parent self.ns = ns self.input = Inputs(values=parent.input._map, ns=ns) @@ -774,10 +772,6 @@ def wrapper(fn: DownloadHandler): return wrapper -def create_ns_func(namespace: str) -> Callable[[str], str]: - return lambda x: namespaced_id_ns(x, [namespace]) - - # ====================================================================================== # Inputs # ====================================================================================== diff --git a/shiny/session/_utils.py b/shiny/session/_utils.py index 1e2372040..e60bb51f3 100644 --- a/shiny/session/_utils.py +++ b/shiny/session/_utils.py @@ -8,6 +8,7 @@ if TYPE_CHECKING: from ._session import Session +from .._namespaces import namespace_context if sys.version_info >= (3, 8): from typing import TypedDict @@ -62,7 +63,8 @@ def session_context(session: Optional["Session"]): """ token: Token[Union[Session, None]] = _current_session.set(session) try: - yield + with namespace_context(session.ns if session else None): + yield finally: _current_session.reset(token) diff --git a/tests/test_modules.py b/tests/test_modules.py index e5af66454..0cf16753a 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -11,7 +11,7 @@ @module_ui -def mod_inner() -> TagList: +def mod_inner_ui() -> TagList: return TagList( ui.input_action_button("button", label="inner"), ui.output_text(namespaced_id("out")), @@ -19,8 +19,8 @@ def mod_inner() -> TagList: @module_ui -def mod_outer() -> TagList: - return TagList(mod_inner("inner"), ui.output_text("out2")) +def mod_outer_ui() -> TagList: + return TagList(mod_inner_ui("inner"), ui.output_text("out2")) def get_id(x: TagList, child_idx: int = 0) -> str: @@ -28,10 +28,10 @@ def get_id(x: TagList, child_idx: int = 0) -> str: def test_module_ui(): - x = mod_inner("inner") + x = mod_inner_ui("inner") assert get_id(x, 0) == "inner_button" assert get_id(x, 1) == "inner_out" - y = mod_outer("outer") + y = mod_outer_ui("outer") assert get_id(y, 0) == "outer_inner_button" assert get_id(y, 1) == "outer_inner_out" assert get_id(y, 2) == "outer_out2" @@ -42,7 +42,7 @@ def test_session_scoping(): sessions: Dict[str, Union[Session, None, str]] = {} @module_server - def inner(input: Inputs, output: Outputs, session: Session): + def inner_server(input: Inputs, output: Outputs, session: Session): @reactive.Calc def out(): return get_current_session() @@ -53,25 +53,25 @@ def _(): sessions["inner_current"] = get_current_session() sessions["inner_calc_current"] = out() sessions["inner_id"] = session.ns("foo") - sessions["inner_ui_id"] = get_id(mod_outer("outer"), 0) + sessions["inner_ui_id"] = get_id(mod_outer_ui("outer"), 0) @module_server - def outer(input: Inputs, output: Outputs, session: Session): + def outer_server(input: Inputs, output: Outputs, session: Session): @reactive.Calc def out(): return get_current_session() @reactive.Effect def _(): - inner("mod_inner") + inner_server("mod_inner") sessions["outer"] = session sessions["outer_current"] = get_current_session() sessions["outer_calc_current"] = out() sessions["outer_id"] = session.ns("foo") - sessions["outer_ui_id"] = get_id(mod_outer("outer"), 0) + sessions["outer_ui_id"] = get_id(mod_outer_ui("outer"), 0) def server(input: Inputs, output: Outputs, session: Session): - outer("mod_outer") + outer_server("mod_outer") @reactive.Calc def out(): @@ -83,7 +83,7 @@ def _(): sessions["top_current"] = get_current_session() sessions["top_calc_current"] = out() sessions["top_id"] = session.ns("foo") - sessions["top_ui_id"] = get_id(mod_outer("outer"), 0) + sessions["top_ui_id"] = get_id(mod_outer_ui("outer"), 0) App(ui.TagList(), server)._create_session(MockConnection()) run_coro_sync(reactive.flush()) @@ -92,13 +92,13 @@ def _(): assert sessions["inner_current"] is sessions["inner_calc_current"] assert isinstance(sessions["inner_current"], Session) assert sessions["inner_id"] == "mod_outer_mod_inner_foo" - assert sessions["inner_ui_id"] == "outer_inner_button" + assert sessions["inner_ui_id"] == "mod_outer_mod_inner_outer_inner_button" assert sessions["outer"] is sessions["outer_current"] assert sessions["outer_current"] is sessions["outer_calc_current"] assert isinstance(sessions["outer_current"], Session) assert sessions["outer_id"] == "mod_outer_foo" - assert sessions["outer_ui_id"] == "outer_inner_button" + assert sessions["outer_ui_id"] == "mod_outer_outer_inner_button" assert sessions["top"] is sessions["top_current"] assert sessions["top_current"] is sessions["top_calc_current"] diff --git a/tests/test_poll.py b/tests/test_poll.py index 13382eb97..01c6b3249 100644 --- a/tests/test_poll.py +++ b/tests/test_poll.py @@ -12,6 +12,7 @@ from shiny import * from shiny import _utils from shiny.reactive import * +from shiny._namespaces import Root from .mocktime import MockTime @@ -25,6 +26,8 @@ class OnEndedSessionCallbacks: Eventually we should have a proper mock of Session, then we can retire this. """ + ns = Root + def __init__(self): self._on_ended_callbacks = _utils.Callbacks() # Unfortunately we have to lie here and say we're a session. Obvously, any From 70885aeff4a0cdf1244edbef59df9ea495a24347 Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Thu, 23 Jun 2022 22:35:23 -0700 Subject: [PATCH 08/13] Rename module_ui to module.ui (& server) and namespaced_id to resolve_id --- examples/moduleapp/app.py | 4 ++-- shiny/__init__.py | 6 +++--- shiny/_namespaces.py | 30 +++++++++++++++++++++++------- shiny/examples/Module/app.py | 4 ++-- shiny/{_modules.py => module.py} | 8 ++++---- shiny/session/_session.py | 26 +++++++++++++------------- shiny/ui/_download_button.py | 6 +++--- shiny/ui/_input_action_button.py | 6 +++--- shiny/ui/_input_check_radio.py | 12 ++++++------ shiny/ui/_input_date.py | 12 ++++++------ shiny/ui/_input_file.py | 4 ++-- shiny/ui/_input_numeric.py | 4 ++-- shiny/ui/_input_password.py | 4 ++-- shiny/ui/_input_select.py | 4 ++-- shiny/ui/_input_slider.py | 4 ++-- shiny/ui/_input_text.py | 6 +++--- shiny/ui/_input_update.py | 8 ++++---- shiny/ui/_navs.py | 16 ++++++++-------- shiny/ui/_output.py | 12 ++++++------ shiny/ui/_page.py | 4 ++-- tests/test_modules.py | 32 ++++++++++++++++---------------- 21 files changed, 114 insertions(+), 98 deletions(-) rename shiny/{_modules.py => module.py} (80%) diff --git a/examples/moduleapp/app.py b/examples/moduleapp/app.py index e90b15a3e..f6e83b995 100644 --- a/examples/moduleapp/app.py +++ b/examples/moduleapp/app.py @@ -3,7 +3,7 @@ # ============================================================ # Counter module # ============================================================ -@module_ui +@module.ui def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: return ui.div( {"style": "border: 1px solid #ccc; border-radius: 5px; margin: 5px 0;"}, @@ -13,7 +13,7 @@ def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: ) -@module_server +@module.server def counter_server( input: Inputs, output: Outputs, session: Session, starting_value: int = 0 ): diff --git a/shiny/__init__.py b/shiny/__init__.py index 597a01f43..40a339e9e 100644 --- a/shiny/__init__.py +++ b/shiny/__init__.py @@ -13,10 +13,11 @@ # Private submodules that have some user-facing functionality from ._app import App from ._decorators import event -from ._modules import module_ui, module_server from ._validation import req from ._deprecated import * +from . import module + if _is_pyodide: # In pyodide, avoid importing _main because it imports packages that aren't # available. @@ -43,8 +44,7 @@ # _main.py "run_app", # _modules.py - "module_ui", - "module_server", + "module", # _session.py "Session", "Inputs", diff --git a/shiny/_namespaces.py b/shiny/_namespaces.py index 5525f9027..bf7a81459 100644 --- a/shiny/_namespaces.py +++ b/shiny/_namespaces.py @@ -1,14 +1,20 @@ from contextlib import contextmanager from contextvars import ContextVar, Token +import re from typing import Union class ResolvedId(str): def __call__(self, id: "Id") -> "ResolvedId": - if self == "" or isinstance(id, ResolvedId): + if isinstance(id, ResolvedId): + return id + + validate_id(id) + + if self == "": return ResolvedId(id) else: - return ResolvedId(self + "_" + id) + return ResolvedId(self + "-" + id) Root = ResolvedId("") @@ -17,12 +23,22 @@ def __call__(self, id: "Id") -> "ResolvedId": Id = Union[str, ResolvedId] -def namespaced_id(id: Id) -> ResolvedId: - return ResolvedId(get_current_namespace())(id) +def resolve_id(id: Id) -> ResolvedId: + curr_ns = _current_namespace.get() + return curr_ns(id) + + +# \w is a large set for unicode patterns, that's fine; we mostly want to avoid some +# special characters like space, comma, period, and especially dash +re_valid_id = re.compile("^\\.?\\w+$") -def get_current_namespace() -> ResolvedId: - return _current_namespace.get() +def validate_id(id: str): + if not re_valid_id.match(id): + raise ValueError( + f"The string '{id}' is not a valid id; only letters, numbers, and " + "underscore are permitted" + ) _current_namespace: ContextVar[ResolvedId] = ContextVar( @@ -32,7 +48,7 @@ def get_current_namespace() -> ResolvedId: @contextmanager def namespace_context(id: Union[Id, None]): - namespace = namespaced_id(id) if id else Root + namespace = resolve_id(id) if id else Root token: Token[ResolvedId] = _current_namespace.set(namespace) try: yield diff --git a/shiny/examples/Module/app.py b/shiny/examples/Module/app.py index e90b15a3e..f6e83b995 100644 --- a/shiny/examples/Module/app.py +++ b/shiny/examples/Module/app.py @@ -3,7 +3,7 @@ # ============================================================ # Counter module # ============================================================ -@module_ui +@module.ui def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: return ui.div( {"style": "border: 1px solid #ccc; border-radius: 5px; margin: 5px 0;"}, @@ -13,7 +13,7 @@ def counter_ui(label: str = "Increment counter") -> ui.TagChildArg: ) -@module_server +@module.server def counter_server( input: Inputs, output: Outputs, session: Session, starting_value: int = 0 ): diff --git a/shiny/_modules.py b/shiny/module.py similarity index 80% rename from shiny/_modules.py rename to shiny/module.py index fe2693efe..d1ddc482a 100644 --- a/shiny/_modules.py +++ b/shiny/module.py @@ -1,4 +1,4 @@ -__all__ = ("namespaced_id", "module_ui", "module_server") +__all__ = ("resolve_id", "ui", "server") import sys from typing import Callable, TypeVar @@ -8,14 +8,14 @@ else: from typing import ParamSpec, Concatenate -from ._namespaces import namespaced_id, namespace_context, Id +from ._namespaces import resolve_id, namespace_context, Id from .session import Inputs, Outputs, Session, require_active_session, session_context P = ParamSpec("P") R = TypeVar("R") -def module_ui(fn: Callable[P, R]) -> Callable[Concatenate[str, P], R]: +def ui(fn: Callable[P, R]) -> Callable[Concatenate[str, P], R]: def wrapper(id: Id, *args: P.args, **kwargs: P.kwargs) -> R: with namespace_context(id): return fn(*args, **kwargs) @@ -23,7 +23,7 @@ def wrapper(id: Id, *args: P.args, **kwargs: P.kwargs) -> R: return wrapper -def module_server( +def server( fn: Callable[Concatenate[Inputs, Outputs, Session, P], R] ) -> Callable[Concatenate[str, P], R]: def wrapper(id: Id, *args: P.args, **kwargs: P.kwargs) -> R: diff --git a/shiny/session/_session.py b/shiny/session/_session.py index ad0591049..b3136f6e1 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -282,7 +282,17 @@ def _manage_inputs(self, data: Dict[str, object]) -> None: self.input[keys[0]]._set(val) - self.output._manage_hidden() + self.output._manage_hidden() + + def _is_hidden(self, name: str) -> bool: + with isolate(): + hidden_value_obj = cast( + Value[bool], self.input[f".clientdata_output_{name}_hidden"] + ) + if not hidden_value_obj.is_set(): + return True + + return hidden_value_obj() # ========================================================================== # Message handlers @@ -906,7 +916,7 @@ def set_fn(fn: render.RenderFunction) -> None: self._suspend_when_hidden[output_name] = suspend_when_hidden @Effect( - suspended=suspend_when_hidden and self._is_hidden(output_name), + suspended=suspend_when_hidden and self._session._is_hidden(output_name), priority=priority, ) async def output_obs(): @@ -975,14 +985,4 @@ def _manage_hidden(self) -> None: self._effects[name].resume() def _should_suspend(self, name: str) -> bool: - return self._suspend_when_hidden[name] and self._is_hidden(name) - - def _is_hidden(self, name: str) -> bool: - with isolate(): - hidden_value_obj = cast( - Value[bool], self._session.input[f".clientdata_output_{name}_hidden"] - ) - if not hidden_value_obj.is_set(): - return True - - return hidden_value_obj() + return self._suspend_when_hidden[name] and self._session._is_hidden(name) diff --git a/shiny/ui/_download_button.py b/shiny/ui/_download_button.py index 70d32c14c..3a85cd937 100644 --- a/shiny/ui/_download_button.py +++ b/shiny/ui/_download_button.py @@ -5,7 +5,7 @@ from htmltools import tags, Tag, TagChildArg, TagAttrArg, css from .._docstring import add_example -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from .._shinyenv import is_pyodide @@ -47,7 +47,7 @@ def download_button( icon, label, {"class": "btn btn-default shiny-download-link", "style": css(width=width)}, - id=namespaced_id(id), + id=resolve_id(id), # This is a fake link that just results in a 404. It will be replaced by a # working link after the server side logic runs, so this link will only be # visited in cases where the user clicks the button too fast, or if the server @@ -100,7 +100,7 @@ def download_link( icon, label, {"class": "shiny-download-link", "style": css(width=width)}, - id=namespaced_id(id), + id=resolve_id(id), href="session/0/download/missing_download", target="_blank", download=None if is_pyodide else True, diff --git a/shiny/ui/_input_action_button.py b/shiny/ui/_input_action_button.py index 6bbe2705c..6703faec4 100644 --- a/shiny/ui/_input_action_button.py +++ b/shiny/ui/_input_action_button.py @@ -5,7 +5,7 @@ from htmltools import tags, Tag, TagChildArg, TagAttrArg, css from .._docstring import add_example -from .._namespaces import namespaced_id +from .._namespaces import resolve_id @add_example() @@ -54,7 +54,7 @@ def input_action_button( {"class": "btn btn-default action-button", "style": css(width=width)}, icon, label, - id=namespaced_id(id), + id=resolve_id(id), type="button", **kwargs, ) @@ -103,7 +103,7 @@ def input_action_link( {"class": "action-button"}, icon, label, - id=namespaced_id(id), + id=resolve_id(id), href="#", **kwargs, ) diff --git a/shiny/ui/_input_check_radio.py b/shiny/ui/_input_check_radio.py index 87b2f4b44..f8c346fb2 100644 --- a/shiny/ui/_input_check_radio.py +++ b/shiny/ui/_input_check_radio.py @@ -10,7 +10,7 @@ from .._docstring import add_example -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ._utils import shiny_input_label # Canonical format for representing select options. @@ -59,7 +59,7 @@ def input_checkbox( div( tags.label( tags.input( - id=namespaced_id(id), + id=resolve_id(id), type="checkbox", checked="checked" if value else None, ), @@ -122,7 +122,7 @@ def input_checkbox_group( input_label = shiny_input_label(id, label) options = _generate_options( - id=namespaced_id(id), + id=resolve_id(id), type="checkbox", choices=choices, selected=selected, @@ -131,7 +131,7 @@ def input_checkbox_group( return div( input_label, options, - id=namespaced_id(id), + id=resolve_id(id), style=css(width=width), class_="form-group shiny-input-checkboxgroup shiny-input-container" + (" shiny-input-container-inline" if inline else ""), @@ -189,7 +189,7 @@ def input_radio_buttons( input_label = shiny_input_label(id, label) options = _generate_options( - id=namespaced_id(id), + id=resolve_id(id), type="radio", choices=choices, selected=selected, @@ -198,7 +198,7 @@ def input_radio_buttons( return div( input_label, options, - id=namespaced_id(id), + id=resolve_id(id), style=css(width=width), class_="form-group shiny-input-radiogroup shiny-input-container" + (" shiny-input-container-inline" if inline else ""), diff --git a/shiny/ui/_input_date.py b/shiny/ui/_input_date.py index 94b538eaf..7abd4d4f3 100644 --- a/shiny/ui/_input_date.py +++ b/shiny/ui/_input_date.py @@ -8,7 +8,7 @@ from .._docstring import add_example from ._html_dependencies import datepicker_deps -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ._utils import shiny_input_label @@ -110,7 +110,7 @@ def input_date( return div( shiny_input_label(id, label), _date_input_tag( - id=namespaced_id(id), + id=resolve_id(id), value=value, min=min, max=max, @@ -122,7 +122,7 @@ def input_date( data_date_dates_disabled=json.dumps(datesdisabled), data_date_days_of_week_disabled=json.dumps(daysofweekdisabled), ), - id=namespaced_id(id), + id=resolve_id(id), class_="shiny-date-input form-group shiny-input-container", style=css(width=width), ) @@ -228,7 +228,7 @@ def input_date_range( shiny_input_label(id, label), div( _date_input_tag( - id=namespaced_id(id), + id=resolve_id(id), value=start, min=min, max=max, @@ -244,7 +244,7 @@ def input_date_range( class_="input-group-addon input-group-prepend input-group-append", ), _date_input_tag( - id=namespaced_id(id), + id=resolve_id(id), value=end, min=min, max=max, @@ -257,7 +257,7 @@ def input_date_range( # input-daterange class is needed for dropdown behavior class_="input-daterange input-group input-group-sm", ), - id=namespaced_id(id), + id=resolve_id(id), class_="shiny-date-range-input form-group shiny-input-container", style=css(width=width), ) diff --git a/shiny/ui/_input_file.py b/shiny/ui/_input_file.py index 62d096207..274768799 100644 --- a/shiny/ui/_input_file.py +++ b/shiny/ui/_input_file.py @@ -11,7 +11,7 @@ from htmltools import Tag, TagChildArg, css, div, span, tags from .._docstring import add_example -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ._utils import shiny_input_label @@ -90,7 +90,7 @@ def input_file( btn_file = span( button_label, tags.input( - id=namespaced_id(id), + id=resolve_id(id), name=id, type="file", multiple="multiple" if multiple else None, diff --git a/shiny/ui/_input_numeric.py b/shiny/ui/_input_numeric.py index ee3444fdc..a1fcf5996 100644 --- a/shiny/ui/_input_numeric.py +++ b/shiny/ui/_input_numeric.py @@ -5,7 +5,7 @@ from htmltools import tags, Tag, div, css, TagChildArg from .._docstring import add_example -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ._utils import shiny_input_label @@ -58,7 +58,7 @@ def input_numeric( return div( shiny_input_label(id, label), tags.input( - id=namespaced_id(id), + id=resolve_id(id), type="number", class_="form-control", value=value, diff --git a/shiny/ui/_input_password.py b/shiny/ui/_input_password.py index 5bbe041cd..8c14e961b 100644 --- a/shiny/ui/_input_password.py +++ b/shiny/ui/_input_password.py @@ -5,7 +5,7 @@ from htmltools import tags, Tag, div, css, TagChildArg from .._docstring import add_example -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ._utils import shiny_input_label @@ -51,7 +51,7 @@ def input_password( return div( shiny_input_label(id, label), tags.input( - id=namespaced_id(id), + id=resolve_id(id), type="password", value=value, class_="form-control", diff --git a/shiny/ui/_input_select.py b/shiny/ui/_input_select.py index 31de8ddd0..ec14a6b9c 100644 --- a/shiny/ui/_input_select.py +++ b/shiny/ui/_input_select.py @@ -9,7 +9,7 @@ from .._docstring import add_example from ._html_dependencies import selectize_deps -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ._utils import shiny_input_label _Choices = Dict[str, TagChildArg] @@ -167,7 +167,7 @@ def input_select( div( tags.select( *choices_tags, - id=namespaced_id(id), + id=resolve_id(id), class_=None if selectize else "form-select", multiple=multiple, width=width, diff --git a/shiny/ui/_input_slider.py b/shiny/ui/_input_slider.py index b91f34811..5db18bfa9 100644 --- a/shiny/ui/_input_slider.py +++ b/shiny/ui/_input_slider.py @@ -14,7 +14,7 @@ from .._docstring import add_example from ._html_dependencies import ionrangeslider_deps -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ._utils import shiny_input_label # Even though TypedDict is available in Python 3.8, because it's used with NotRequired, @@ -170,7 +170,7 @@ def input_slider( scale_factor = math.ceil(n_steps / 10) n_ticks = n_steps / scale_factor - id = namespaced_id(id) + id = resolve_id(id) props: Dict[str, TagAttrArg] = { "class_": "js-range-slider", diff --git a/shiny/ui/_input_text.py b/shiny/ui/_input_text.py index f1f2cd88a..72615732c 100644 --- a/shiny/ui/_input_text.py +++ b/shiny/ui/_input_text.py @@ -11,7 +11,7 @@ from htmltools import Tag, TagChildArg, css, div, tags from .._docstring import add_example -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ._utils import shiny_input_label @@ -70,7 +70,7 @@ def input_text( return div( shiny_input_label(id, label), tags.input( - id=namespaced_id(id), + id=resolve_id(id), type="text", class_="form-control", value=value, @@ -158,7 +158,7 @@ def input_text_area( area = tags.textarea( value, - id=namespaced_id(id), + id=resolve_id(id), class_="form-control", style=css(width=None if width else "100%", height=height, resize=resize), placeholder=placeholder, diff --git a/shiny/ui/_input_update.py b/shiny/ui/_input_update.py index f241ea048..9a6ed93cb 100644 --- a/shiny/ui/_input_update.py +++ b/shiny/ui/_input_update.py @@ -41,7 +41,7 @@ from ._input_date import _as_date_attr from ._input_select import SelectChoicesArg, _normalize_choices, _render_choices from ._input_slider import SliderValueArg, SliderStepArg, _slider_type, _as_numeric -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from .._utils import drop_none from ..session import Session, require_active_session @@ -193,7 +193,7 @@ def update_checkbox_group( """ _update_choice_input( - id=namespaced_id(id), + id=resolve_id(id), type="checkbox", label=label, choices=choices, @@ -245,7 +245,7 @@ def update_radio_buttons( """ _update_choice_input( - id=namespaced_id(id), + id=resolve_id(id), type="radio", label=label, choices=choices, @@ -269,7 +269,7 @@ def _update_choice_input( options = None if choices is not None: opts = _generate_options( - id=namespaced_id(id), + id=resolve_id(id), type=type, choices=choices, selected=selected, diff --git a/shiny/ui/_navs.py b/shiny/ui/_navs.py index 399b411a6..78b0852aa 100644 --- a/shiny/ui/_navs.py +++ b/shiny/ui/_navs.py @@ -27,7 +27,7 @@ from ._bootstrap import row, column from .._docstring import add_example from ._html_dependencies import bootstrap_deps -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ..types import NavSetArg from .._utils import private_random_int @@ -409,7 +409,7 @@ def navset_tab( return NavSet( *args, ul_class="nav nav-tabs", - id=namespaced_id(id) if id else None, + id=resolve_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -461,7 +461,7 @@ def navset_pill( return NavSet( *args, ul_class="nav nav-pills", - id=namespaced_id(id) if id else None, + id=resolve_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -511,7 +511,7 @@ def navset_hidden( return NavSet( *args, ul_class="nav nav-hidden", - id=namespaced_id(id) if id else None, + id=resolve_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -591,7 +591,7 @@ def navset_tab_card( return NavSetCard( *args, ul_class="nav nav-tabs card-header-tabs", - id=namespaced_id(id) if id else None, + id=resolve_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -647,7 +647,7 @@ def navset_pill_card( return NavSetCard( *args, ul_class="nav nav-pills card-header-pills", - id=namespaced_id(id) if id else None, + id=resolve_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -741,7 +741,7 @@ def navset_pill_list( return NavSetPillList( *args, ul_class="nav nav-pills nav-stacked", - id=namespaced_id(id) if id else None, + id=resolve_id(id) if id else None, selected=selected, header=header, footer=footer, @@ -902,7 +902,7 @@ def navset_bar( return NavSetBar( *args, ul_class="nav navbar-nav", - id=namespaced_id(id) if id else None, + id=resolve_id(id) if id else None, selected=selected, title=title, position=position, diff --git a/shiny/ui/_output.py b/shiny/ui/_output.py index 5c633ed63..782153263 100644 --- a/shiny/ui/_output.py +++ b/shiny/ui/_output.py @@ -10,7 +10,7 @@ from htmltools import tags, Tag, div, css, TagAttrArg, TagFunction from .._docstring import add_example -from .._namespaces import namespaced_id +from .._namespaces import resolve_id @add_example() @@ -43,7 +43,7 @@ def output_plot( ~shiny.render.plot ~shiny.ui.output_image """ - res = output_image(id=namespaced_id(id), width=width, height=height, inline=inline) + res = output_image(id=resolve_id(id), width=width, height=height, inline=inline) res.add_class("shiny-plot-output") return res @@ -77,7 +77,7 @@ def output_image( """ func = tags.span if inline else div style = None if inline else css(width=width, height=height) - return func(id=namespaced_id(id), class_="shiny-image-output", style=style) + return func(id=resolve_id(id), class_="shiny-image-output", style=style) @add_example() @@ -112,7 +112,7 @@ def output_text( if not container: container = tags.span if inline else tags.div - return container(id=namespaced_id(id), class_="shiny-text-output") + return container(id=resolve_id(id), class_="shiny-text-output") def output_text_verbatim(id: str, placeholder: bool = False) -> Tag: @@ -146,7 +146,7 @@ def output_text_verbatim(id: str, placeholder: bool = False) -> Tag: """ cls = "shiny-text-output" + (" noplaceholder" if not placeholder else "") - return tags.pre(id=namespaced_id(id), class_=cls) + return tags.pre(id=resolve_id(id), class_=cls) @add_example() @@ -182,4 +182,4 @@ def output_ui( if not container: container = tags.span if inline else tags.div - return container({"class": "shiny-html-output"}, id=namespaced_id(id), **kwargs) + return container({"class": "shiny-html-output"}, id=resolve_id(id), **kwargs) diff --git a/shiny/ui/_page.py b/shiny/ui/_page.py index 238532cad..176629050 100644 --- a/shiny/ui/_page.py +++ b/shiny/ui/_page.py @@ -28,7 +28,7 @@ from .._docstring import add_example from ._html_dependencies import bootstrap_deps from ._navs import navset_bar -from .._namespaces import namespaced_id +from .._namespaces import resolve_id from ..types import MISSING, MISSING_TYPE, NavSetArg from ._utils import get_window_title @@ -115,7 +115,7 @@ def page_navbar( navset_bar( *args, title=title, - id=namespaced_id(id) if id else None, + id=resolve_id(id) if id else None, selected=selected, position=position, header=header, diff --git a/tests/test_modules.py b/tests/test_modules.py index 0cf16753a..ca519bef4 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -5,20 +5,20 @@ from shiny import * from shiny.session import get_current_session from shiny._connection import MockConnection -from shiny._namespaces import namespaced_id +from shiny._namespaces import resolve_id from shiny._utils import run_coro_sync from htmltools import TagList, Tag -@module_ui +@module.ui def mod_inner_ui() -> TagList: return TagList( ui.input_action_button("button", label="inner"), - ui.output_text(namespaced_id("out")), + ui.output_text(resolve_id("out")), ) -@module_ui +@module.ui def mod_outer_ui() -> TagList: return TagList(mod_inner_ui("inner"), ui.output_text("out2")) @@ -29,19 +29,19 @@ def get_id(x: TagList, child_idx: int = 0) -> str: def test_module_ui(): x = mod_inner_ui("inner") - assert get_id(x, 0) == "inner_button" - assert get_id(x, 1) == "inner_out" + assert get_id(x, 0) == "inner-button" + assert get_id(x, 1) == "inner-out" y = mod_outer_ui("outer") - assert get_id(y, 0) == "outer_inner_button" - assert get_id(y, 1) == "outer_inner_out" - assert get_id(y, 2) == "outer_out2" + assert get_id(y, 0) == "outer-inner-button" + assert get_id(y, 1) == "outer-inner-out" + assert get_id(y, 2) == "outer-out2" def test_session_scoping(): sessions: Dict[str, Union[Session, None, str]] = {} - @module_server + @module.server def inner_server(input: Inputs, output: Outputs, session: Session): @reactive.Calc def out(): @@ -55,7 +55,7 @@ def _(): sessions["inner_id"] = session.ns("foo") sessions["inner_ui_id"] = get_id(mod_outer_ui("outer"), 0) - @module_server + @module.server def outer_server(input: Inputs, output: Outputs, session: Session): @reactive.Calc def out(): @@ -91,17 +91,17 @@ def _(): assert sessions["inner"] is sessions["inner_current"] assert sessions["inner_current"] is sessions["inner_calc_current"] assert isinstance(sessions["inner_current"], Session) - assert sessions["inner_id"] == "mod_outer_mod_inner_foo" - assert sessions["inner_ui_id"] == "mod_outer_mod_inner_outer_inner_button" + assert sessions["inner_id"] == "mod_outer-mod_inner-foo" + assert sessions["inner_ui_id"] == "mod_outer-mod_inner-outer-inner-button" assert sessions["outer"] is sessions["outer_current"] assert sessions["outer_current"] is sessions["outer_calc_current"] assert isinstance(sessions["outer_current"], Session) - assert sessions["outer_id"] == "mod_outer_foo" - assert sessions["outer_ui_id"] == "mod_outer_outer_inner_button" + assert sessions["outer_id"] == "mod_outer-foo" + assert sessions["outer_ui_id"] == "mod_outer-outer-inner-button" assert sessions["top"] is sessions["top_current"] assert sessions["top_current"] is sessions["top_calc_current"] assert isinstance(sessions["top_current"], Session) assert sessions["top_id"] == "foo" - assert sessions["top_ui_id"] == "outer_inner_button" + assert sessions["top_ui_id"] == "outer-inner-button" From 8f4ec1f9c3aaaa20edeb4bfee77229909345bb20 Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Thu, 23 Jun 2022 23:39:44 -0700 Subject: [PATCH 09/13] Invoke server func after init values received --- shiny/_connection.py | 20 +++++++++----- shiny/session/_session.py | 57 ++++++++++++++++++++++++++------------- tests/test_modules.py | 21 ++++++++++----- 3 files changed, 67 insertions(+), 31 deletions(-) diff --git a/shiny/_connection.py b/shiny/_connection.py index e73d563aa..906ab9a1e 100644 --- a/shiny/_connection.py +++ b/shiny/_connection.py @@ -34,18 +34,16 @@ def __init__(self): # make those more configurable if we need to customize the HTTPConnection (like # "scheme", "path", and "query_string"). self._http_conn = HTTPConnection(scope={"type": "websocket", "headers": {}}) + self._queue: asyncio.Queue[str] = asyncio.Queue() async def send(self, message: str) -> None: pass - # I should say I’m not 100% that the receive method can be a no-op for our testing - # purposes. It might need to be asyncio.sleep(0), and/or it might need an external - # way to yield until we tell the connection to continue, so that the run loop can - # continue. async def receive(self) -> str: - # Sleep forever - await asyncio.Event().wait() - raise RuntimeError("make the type checker happy") + msg = await self._queue.get() + if msg == "": + raise ConnectionClosed() + return msg async def close(self, code: int, reason: Optional[str]) -> None: pass @@ -53,6 +51,14 @@ async def close(self, code: int, reason: Optional[str]) -> None: def get_http_conn(self) -> HTTPConnection: return self._http_conn + def cause_receive(self, message: str) -> None: + """Call from tests to simulate the other side sending a message""" + self._queue.put_nowait(message) + + def cause_disconnect(self) -> None: + """Call from tests to simulate the other side disconnecting""" + self.cause_receive("") + class StarletteConnection(Connection): def __init__(self, conn: starlette.websockets.WebSocket): diff --git a/shiny/session/_session.py b/shiny/session/_session.py index b3136f6e1..648af652a 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -1,5 +1,6 @@ __all__ = ("Session", "Inputs", "Outputs") +import enum import functools import os from pathlib import Path @@ -58,6 +59,19 @@ from .. import render from .. import _utils + +class ConnectionState(enum.Enum): + Start = 0 + Running = 1 + Closed = 2 + + +class ProtocolError(Exception): + def __init__(self, message: str = ""): + super(ProtocolError, self).__init__(message) + self.message = message + + # This cast is necessary because if the type checker thinks that if # "tag" isn't in `message`, then it's not a ClientMessage object. # This will be fixable when TypedDict items can be marked as @@ -186,9 +200,6 @@ def __init__( self._flush_callbacks = _utils.Callbacks() self._flushed_callbacks = _utils.Callbacks() - with session_context(self): - self.app.server(self.input, self.output, self) - def _register_session_end_callbacks(self) -> None: # This is to be called from the initialization. It registers functions # that are called when a session ends. @@ -213,6 +224,12 @@ async def close(self, code: int = 1001) -> None: self._run_session_end_tasks() async def _run(self) -> None: + conn_state: ConnectionState = ConnectionState.Start + + def verify_state(expected_state: ConnectionState) -> None: + if conn_state != expected_state: + raise ProtocolError("Invalid method for the current session state") + await self._send_message( {"config": {"workerId": "", "sessionId": str(self.id), "user": None}} ) @@ -228,8 +245,8 @@ async def _run(self) -> None: message, object_hook=_utils.lists_to_tuples ) except json.JSONDecodeError: - print("ERROR: Invalid JSON message") - continue + warnings.warn("ERROR: Invalid JSON message") + return if "method" not in message_obj: self._send_error_response("Message does not contain 'method'.") @@ -238,36 +255,40 @@ async def _run(self) -> None: async with lock(): if message_obj["method"] == "init": + verify_state(ConnectionState.Start) + + conn_state = ConnectionState.Running message_obj = typing.cast(ClientMessageInit, message_obj) self._manage_inputs(message_obj["data"]) + with session_context(self): + self.app.server(self.input, self.output, self) + elif message_obj["method"] == "update": + verify_state(ConnectionState.Running) + message_obj = typing.cast(ClientMessageUpdate, message_obj) self._manage_inputs(message_obj["data"]) - else: - if "tag" not in message_obj: - warnings.warn( - "Cannot dispatch message with missing 'tag'; method: " - + message_obj["method"] - ) - return - if "args" not in message_obj: - warnings.warn( - "Cannot dispatch message with missing 'args'; method: " - + message_obj["method"] - ) - return + elif "tag" in message_obj and "args" in message_obj: + verify_state(ConnectionState.Running) message_obj = typing.cast(ClientMessageOther, message_obj) await self._dispatch(message_obj) + else: + raise ProtocolError( + f"Unrecognized method {message_obj['method']}" + ) + self._request_flush() await flush() except ConnectionClosed: self._run_session_end_tasks() + except ProtocolError as pe: + self._send_error_response(pe.message) def _manage_inputs(self, data: Dict[str, object]) -> None: for (key, val) in data.items(): diff --git a/tests/test_modules.py b/tests/test_modules.py index ca519bef4..f526150d8 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -1,13 +1,15 @@ """Tests for `Module`.""" +import asyncio from typing import Dict, Union, cast +import pytest + +from htmltools import Tag, TagList from shiny import * -from shiny.session import get_current_session from shiny._connection import MockConnection from shiny._namespaces import resolve_id -from shiny._utils import run_coro_sync -from htmltools import TagList, Tag +from shiny.session import get_current_session @module.ui @@ -37,7 +39,8 @@ def test_module_ui(): assert get_id(y, 2) == "outer-out2" -def test_session_scoping(): +@pytest.mark.asyncio +async def test_session_scoping(): sessions: Dict[str, Union[Session, None, str]] = {} @@ -85,8 +88,14 @@ def _(): sessions["top_id"] = session.ns("foo") sessions["top_ui_id"] = get_id(mod_outer_ui("outer"), 0) - App(ui.TagList(), server)._create_session(MockConnection()) - run_coro_sync(reactive.flush()) + conn = MockConnection() + sess = App(ui.TagList(), server)._create_session(conn) + + async def mock_client(): + conn.cause_receive('{"method":"init","data":{}}') + conn.cause_disconnect() + + await asyncio.gather(mock_client(), sess._run()) assert sessions["inner"] is sessions["inner_current"] assert sessions["inner_current"] is sessions["inner_calc_current"] From 831d522095305fe281fc8334d28e5d70033e12e9 Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Fri, 24 Jun 2022 13:30:09 -0700 Subject: [PATCH 10/13] Avoid mutable objects as function defaults; use Root as root ns --- shiny/session/_session.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index 648af652a..87a1a618b 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -162,8 +162,8 @@ def __init__( # query information about the request, like headers, cookies, etc. self.http_conn: HTTPConnection = conn.get_http_conn() - self.input: Inputs = Inputs() - self.output: Outputs = Outputs(self) + self.input: Inputs = Inputs(dict()) + self.output: Outputs = Outputs(self, self.ns, dict(), dict()) self.user: Union[str, None] = None self.groups: Union[List[str], None] = None @@ -301,14 +301,20 @@ def _manage_inputs(self, data: Dict[str, object]) -> None: if len(keys) == 2: val = input_handlers._process_value(keys[1], val, keys[0], self) - self.input[keys[0]]._set(val) + # The keys[0] value is already a fully namespaced id; make that explicit by + # wrapping it in ResolvedId, otherwise self.input will throw an id + # validation error. + self.input[ResolvedId(keys[0])]._set(val) self.output._manage_hidden() def _is_hidden(self, name: str) -> bool: with isolate(): + # The .clientdata_output_{name}_hidden string is already a fully namespaced + # id; make that explicit by wrapping it in ResolvedId, otherwise self.input + # will throw an id validation error. hidden_value_obj = cast( - Value[bool], self.input[f".clientdata_output_{name}_hidden"] + Value[bool], self.input[ResolvedId(f".clientdata_output_{name}_hidden")] ) if not hidden_value_obj.is_set(): return True @@ -365,7 +371,10 @@ async def uploadEnd(job_id: str, input_id: str) -> None: ) return None file_data = upload_op.finish() - self.input[input_id]._set(file_data) + # The input_id string is already a fully namespaced id; make that explicit + # by wrapping it in ResolvedId, otherwise self.input will throw an id + # validation error. + self.input[ResolvedId(input_id)]._set(file_data) # Explicitly return None to signal that the message was handled. return None @@ -822,7 +831,7 @@ class Inputs: """ def __init__( - self, values: Dict[str, Value[Any]] = {}, ns: Callable[[str], str] = lambda x: x + self, values: Dict[str, Value[Any]], ns: Callable[[str], str] = Root ) -> None: self._map = values self._ns = ns @@ -881,9 +890,9 @@ class Outputs: def __init__( self, session: Session, - ns: Callable[[str], str] = lambda x: x, - effects: Dict[str, Effect_] = {}, - suspend_when_hidden: Dict[str, bool] = {}, + ns: Callable[[str], str], + effects: Dict[str, Effect_], + suspend_when_hidden: Dict[str, bool], ) -> None: self._session = session self._ns = ns From e505d10ccbd065fcf15f4fce4c54737b1ea1744b Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Fri, 24 Jun 2022 13:43:55 -0700 Subject: [PATCH 11/13] Code review feedback: warn every time --- shiny/session/_session.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index 87a1a618b..cde2c4f27 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -72,6 +72,14 @@ def __init__(self, message: str = ""): self.message = message +class SessionWarning(RuntimeWarning): + pass + + +# By default warnings are shown once; we want to always show them. +warnings.simplefilter("always", SessionWarning) + + # This cast is necessary because if the type checker thinks that if # "tag" isn't in `message`, then it's not a ClientMessage object. # This will be fixable when TypedDict items can be marked as @@ -245,7 +253,7 @@ def verify_state(expected_state: ConnectionState) -> None: message, object_hook=_utils.lists_to_tuples ) except json.JSONDecodeError: - warnings.warn("ERROR: Invalid JSON message") + warnings.warn("ERROR: Invalid JSON message", SessionWarning) return if "method" not in message_obj: @@ -367,7 +375,8 @@ async def uploadEnd(job_id: str, input_id: str) -> None: upload_op = self._file_upload_manager.get_upload_operation(job_id) if upload_op is None: warnings.warn( - "Received uploadEnd message for non-existent upload operation." + "Received uploadEnd message for non-existent upload operation.", + SessionWarning, ) return None file_data = upload_op.finish() @@ -425,7 +434,8 @@ async def _handle_request( "Unable to infer a filename for the " f"'{download_id}' download handler; please use " "@session.download(filename=) to specify one " - "manually" + "manually", + SessionWarning, ) filename = download_id From d8735fde7372a4d53469b47d24ff72fd66062244 Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Fri, 24 Jun 2022 20:36:15 -0700 Subject: [PATCH 12/13] Add unit test for namespaces --- tests/test_namespaces.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/test_namespaces.py diff --git a/tests/test_namespaces.py b/tests/test_namespaces.py new file mode 100644 index 000000000..adfdc95c3 --- /dev/null +++ b/tests/test_namespaces.py @@ -0,0 +1,39 @@ +from shiny._namespaces import namespace_context, resolve_id + + +def test_namespaces(): + outer = resolve_id("outer") + assert outer == "outer" + + with namespace_context(outer): + # Check if the namespace_context ("outer") is respected during resolve_id + inner = resolve_id("inner") + assert inner == "outer-inner" + + # You can also use a ResolvedId as a namespace just by calling it with an id str + assert outer("inner") == "outer-inner" + + # If an id is already resolved (based on ResolvedId class), resolving it further + # does nothing + assert resolve_id(outer) == "outer" + + # When namespace contexts are stacked, inner one wins + with namespace_context(inner): + assert resolve_id("inmost") == "outer-inner-inmost" + + # Namespace contexts nest with existing context when string is used + with namespace_context("inner"): + assert resolve_id("inmost") == "outer-inner-inmost" + + # Re-installing the same context as is already in place + with namespace_context(outer): + assert resolve_id("inmost") == "outer-inmost" + + # You can remove the context with None or "" + with namespace_context(None): + assert resolve_id("foo") == "foo" + with namespace_context(""): + assert resolve_id("foo") == "foo" + + # Check that this still works after another context was installed/removed + assert resolve_id("inner") == "outer-inner" From a5397ae5cf308620de6f541761ba53077ca274b1 Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Fri, 24 Jun 2022 20:49:29 -0700 Subject: [PATCH 13/13] Modify moduleapp example to be dynamic --- examples/moduleapp/app.py | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/examples/moduleapp/app.py b/examples/moduleapp/app.py index f6e83b995..66519889c 100644 --- a/examples/moduleapp/app.py +++ b/examples/moduleapp/app.py @@ -30,18 +30,48 @@ def out() -> str: return f"Click count is {count()}" +# ============================================================ +# Counter Wrapper module -- shows that counter still works +# the same way when wrapped in a dynamic UI +# ============================================================ +@module.ui +def counter_wrapper_ui() -> ui.TagChildArg: + return ui.output_ui("dynamic_counter") + + +@module.server +def counter_wrapper_server( + input: Inputs, output: Outputs, session: Session, label: str = "Increment counter" +): + @output() + @render.ui() + def dynamic_counter(): + return counter_ui("counter", label) + + counter_server("counter") + + # ============================================================================= # App that uses module # ============================================================================= app_ui = ui.page_fluid( counter_ui("counter1", "Counter 1"), - counter_ui("counter2", "Counter 2"), + counter_wrapper_ui("counter2_wrapper"), + ui.output_ui("counter3_ui"), ) def server(input: Inputs, output: Outputs, session: Session): counter_server("counter1") - counter_server("counter2") + counter_wrapper_server("counter2_wrapper", "Counter 2") + + @output() + @render.ui() + def counter3_ui(): + counter_server("counter3") + return counter_ui("counter3", "Counter 3") + + counter_server("counter") app = App(app_ui, server)