From 684ba6ee4382ad051e85ca7a007eb1ba8c59ff40 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Thu, 6 Aug 2026 13:48:27 -0400 Subject: [PATCH 01/13] fix: Coerce non-string `selected` in checkbox/radio update functions `ui.input_checkbox_group()` and `ui.input_radio_buttons()` accept `choices` dictionaries with non-string keys (e.g. `dict[int, str]`) and non-string `selected` values, because both end up in HTML `value` attributes, where htmltools stringifies them. `ui.update_checkbox_group()` and `ui.update_radio_buttons()` did not: they forwarded `selected` verbatim as the message's `value`, so the client received `[0, 1, 2]` instead of `["0", "1", "2"]`. The checkbox/radio input bindings pass that value through `$escape()`, which calls `String.prototype.replace()` on it and throws on a number -- after `setValue()` has already cleared every checkbox -- so all options silently ended up unselected. Normalize in the shared pipeline instead of at the edge: `_normalize_choices()` now coerces choice keys to `str`, a new `_normalize_selected()` coerces `selected` the same way (preserving scalar vs. sequence shape so radio inputs still get a scalar `value`), and `_update_choice_input()` sends the normalized value. Both `_generate_options()` callers -- the input constructors and the update functions -- now compare and emit the same string form. Fixes #2272 --- CHANGELOG.md | 2 + shiny/ui/_input_check_radio.py | 46 ++++++-- shiny/ui/_input_update.py | 13 ++- tests/pytest/test_input_check_radio.py | 150 +++++++++++++++++++++++++ 4 files changed, 197 insertions(+), 14 deletions(-) create mode 100644 tests/pytest/test_input_check_radio.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 057f47d4a..59090290e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * When `@expressify` cannot locate a function's definition, the error now names the function as it appears in the source (rather than a `__name__` a decorator may have rewritten), points at the file and line it looked at, and lists the likely causes — an `async def`, a decorator below `expressify()` that returns a wrapper instead of the original function, or a source file modified after import. (#2016) +* `ui.update_checkbox_group()` and `ui.update_radio_buttons()` now accept non-string `selected` values (e.g. the `int` keys of a `dict[int, str]` passed as `choices`), matching what `ui.input_checkbox_group()` and `ui.input_radio_buttons()` already allowed. Choice values always reach the browser as strings (they become HTML `value` attributes), but the update functions forwarded `selected` unchanged, so the client-side value matching threw and left every option unselected — silently, and only on update. (#2413) + ## [1.7.0] - 2026-07-28 ### New features diff --git a/shiny/ui/_input_check_radio.py b/shiny/ui/_input_check_radio.py index 12a1d0958..5f2a8eb53 100644 --- a/shiny/ui/_input_check_radio.py +++ b/shiny/ui/_input_check_radio.py @@ -322,17 +322,18 @@ def _generate_options( inline: bool, ) -> Tag: choicez = _normalize_choices(choices) + selectedz = _normalize_selected(selected) - if selected is None: + selected_list: list[str] + if selectedz is None: if type == "radio": - selected = list(choicez.keys())[0] + selected_list = [list(choicez.keys())[0]] else: - selected = [] - - if isinstance(selected, tuple): - selected = list(selected) - elif not isinstance(selected, list): - selected = [selected] + selected_list = [] + elif isinstance(selectedz, list): + selected_list = selectedz + else: + selected_list = [selectedz] return div( [ @@ -341,7 +342,7 @@ def _generate_options( type, value=choice[0], label=choice[1], - checked=choice[0] in selected, + checked=choice[0] in selected_list, inline=inline, ) for choice in choicez.items() @@ -380,7 +381,30 @@ def _generate_option( def _normalize_choices(x: ChoicesArg) -> _Choices: + # Choice values end up in an HTML `value` attribute, so they are always strings by + # the time they reach the client. Coerce them here (e.g. the `int` keys of a + # `dict[int, str]`) so that server-side comparisons against `selected` -- and the + # `value` we send in `update_*()` messages -- use the same string form. if isinstance(x, (list, tuple)): - return {k: k for k in x} + return {str(k): k for k in x} + else: + return {str(k): v for k, v in x.items()} + + +def _normalize_selected( + x: Optional[str | list[str] | tuple[str, ...]], +) -> Optional[str | list[str]]: + """ + Coerce ``selected`` value(s) to string(s), preserving scalar vs. sequence shape. + + ``selected`` values are matched against choice values, which + :func:`_normalize_choices` normalizes to strings, so they must be normalized the + same way. Tuples become lists so the result is JSON-serializable for + ``update_*()`` messages. + """ + if x is None: + return None + elif isinstance(x, (list, tuple)): + return [str(v) for v in x] else: - return x + return str(x) diff --git a/shiny/ui/_input_update.py b/shiny/ui/_input_update.py index ccbdac4a8..6f835ea3e 100644 --- a/shiny/ui/_input_update.py +++ b/shiny/ui/_input_update.py @@ -36,7 +36,7 @@ from ..module import ResolvedId, resolve_id from ..session import require_active_session, session_context from ..types import ActionButtonValue -from ._input_check_radio import ChoicesArg, _generate_options +from ._input_check_radio import ChoicesArg, _generate_options, _normalize_selected from ._input_date import _as_date_attr from ._input_select import SelectChoicesArg, _normalize_choices, _render_choices from ._input_slider import SliderStepArg, SliderValueArg, _as_numeric, _slider_type @@ -421,6 +421,13 @@ def _update_choice_input( session: Optional[Session] = None, ) -> None: session = require_active_session(session) + + # The client matches `value` against the HTML `value` attributes of the options, + # which are always strings. Normalize the same way `input_checkbox_group()` / + # `input_radio_buttons()` do so that, e.g., the `int` keys of a `dict[int, str]` + # work here too. https://github.com/posit-dev/py-shiny/issues/2272 + selected_values = _normalize_selected(selected) + options = None if choices is not None: # https://github.com/posit-dev/py-shiny/issues/708#issuecomment-1696352934 @@ -431,14 +438,14 @@ def _update_choice_input( id=resolved_id, type=type, choices=choices, - selected=selected, + selected=selected_values, inline=inline, ) options = session._process_ui(opts)["html"] msg = { "label": session._process_ui(label) if label is not None else None, "options": options, - "value": selected, + "value": selected_values, } session.send_input_message(id, drop_none(msg)) diff --git a/tests/pytest/test_input_check_radio.py b/tests/pytest/test_input_check_radio.py new file mode 100644 index 000000000..f1783b073 --- /dev/null +++ b/tests/pytest/test_input_check_radio.py @@ -0,0 +1,150 @@ +""" +Tests for `input_checkbox_group()` / `input_radio_buttons()` and their `update_*()` +counterparts. + +The `update_*()` functions must coerce choice values to strings the same way the +input constructors do, so that `choices={0: "a"}, selected=[0]` behaves identically +to `choices={"0": "a"}, selected=["0"]` (https://github.com/posit-dev/py-shiny/issues/2272). +""" + +from __future__ import annotations + +from typing import Any + +import pytest +from htmltools import TagChild, TagList + +from shiny import ui +from shiny.express._stub_session import ExpressStubSession +from shiny.session import Session, session_context +from shiny.session._session import RenderedDeps + +CHOICES_INT: dict[int, str] = {0: "a", 1: "b", 2: "c"} +CHOICES_STR: dict[str, str] = {"0": "a", "1": "b", "2": "c"} + + +class _MessageCapturingSession(ExpressStubSession): + """Stub session that records `send_input_message()` payloads and renders UI.""" + + def __init__(self) -> None: + super().__init__() + self.messages: list[dict[str, object]] = [] + + def send_input_message(self, id: str, message: dict[str, object]) -> None: + self.messages.append(message) + + def _process_ui(self, ui: TagChild) -> RenderedDeps: + return {"deps": [], "html": TagList(ui).get_html_string()} + + +@pytest.fixture +def session() -> _MessageCapturingSession: + return _MessageCapturingSession() + + +def _send(session: Session, fn: Any, **kwargs: Any) -> dict[str, object]: + with session_context(session): + fn("x", **kwargs) + messages = session.messages # pyright: ignore[reportAttributeAccessIssue] + assert len(messages) == 1 + return messages.pop() + + +# ============================================================================ +# input_checkbox_group() / input_radio_buttons(): int keys work today +# ============================================================================ +def test_input_checkbox_group_int_keys_match_str_keys(): + int_html = ui.input_checkbox_group( + "x", "L", CHOICES_INT, selected=[0, 2] # pyright: ignore[reportArgumentType] + ).get_html_string() + str_html = ui.input_checkbox_group( + "x", "L", CHOICES_STR, selected=["0", "2"] + ).get_html_string() + assert int_html == str_html + assert int_html.count('checked="checked"') == 2 + + +def test_input_radio_buttons_int_keys_match_str_keys(): + int_html = ui.input_radio_buttons( + "x", "L", CHOICES_INT, selected=1 # pyright: ignore[reportArgumentType] + ).get_html_string() + str_html = ui.input_radio_buttons( + "x", "L", CHOICES_STR, selected="1" + ).get_html_string() + assert int_html == str_html + + +# ============================================================================ +# update_checkbox_group(): the wire payload must match the string-keyed one +# ============================================================================ +def test_update_checkbox_group_int_keys_match_str_keys( + session: _MessageCapturingSession, +): + int_msg = _send( + session, + ui.update_checkbox_group, + choices=CHOICES_INT, + selected=list(CHOICES_INT.keys()), + ) + str_msg = _send( + session, + ui.update_checkbox_group, + choices=CHOICES_STR, + selected=list(CHOICES_STR.keys()), + ) + assert int_msg == str_msg + # The client matches `value` against the HTML `value` attributes, which are + # always strings. + assert int_msg["value"] == ["0", "1", "2"] + + +def test_update_checkbox_group_int_selected_without_choices( + session: _MessageCapturingSession, +): + msg = _send(session, ui.update_checkbox_group, selected=[0, 2]) + assert msg == {"value": ["0", "2"]} + + +def test_update_checkbox_group_selected_tuple_becomes_list_of_str( + session: _MessageCapturingSession, +): + msg = _send(session, ui.update_checkbox_group, selected=(0, 1)) + assert msg == {"value": ["0", "1"]} + + +def test_update_checkbox_group_scalar_selected_stays_scalar( + session: _MessageCapturingSession, +): + # A scalar `selected` must not become a list -- the client handles both, but + # the shape should round-trip unchanged (apart from the `str()` coercion). + msg = _send(session, ui.update_checkbox_group, selected=1) + assert msg == {"value": "1"} + + +# ============================================================================ +# update_radio_buttons(): same defect, same file +# ============================================================================ +def test_update_radio_buttons_int_keys_match_str_keys( + session: _MessageCapturingSession, +): + int_msg = _send(session, ui.update_radio_buttons, choices=CHOICES_INT, selected=1) + str_msg = _send(session, ui.update_radio_buttons, choices=CHOICES_STR, selected="1") + assert int_msg == str_msg + assert int_msg["value"] == "1" + + +def test_update_radio_buttons_int_selected_without_choices( + session: _MessageCapturingSession, +): + msg = _send(session, ui.update_radio_buttons, selected=2) + assert msg == {"value": "2"} + + +# ============================================================================ +# `selected=None` must stay absent from the payload (drop_none) +# ============================================================================ +def test_update_choice_input_none_selected_is_dropped( + session: _MessageCapturingSession, +): + msg = _send(session, ui.update_checkbox_group, label="New label") + assert "value" not in msg From e9bf49e4dc834f99532ff7638847daad499d6b50 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Thu, 6 Aug 2026 13:49:29 -0400 Subject: [PATCH 02/13] docs: Reference PR number in changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59090290e..4e6f9e606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,7 +49,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * When `@expressify` cannot locate a function's definition, the error now names the function as it appears in the source (rather than a `__name__` a decorator may have rewritten), points at the file and line it looked at, and lists the likely causes — an `async def`, a decorator below `expressify()` that returns a wrapper instead of the original function, or a source file modified after import. (#2016) -* `ui.update_checkbox_group()` and `ui.update_radio_buttons()` now accept non-string `selected` values (e.g. the `int` keys of a `dict[int, str]` passed as `choices`), matching what `ui.input_checkbox_group()` and `ui.input_radio_buttons()` already allowed. Choice values always reach the browser as strings (they become HTML `value` attributes), but the update functions forwarded `selected` unchanged, so the client-side value matching threw and left every option unselected — silently, and only on update. (#2413) +* `ui.update_checkbox_group()` and `ui.update_radio_buttons()` now accept non-string `selected` values (e.g. the `int` keys of a `dict[int, str]` passed as `choices`), matching what `ui.input_checkbox_group()` and `ui.input_radio_buttons()` already allowed. Choice values always reach the browser as strings (they become HTML `value` attributes), but the update functions forwarded `selected` unchanged, so the client-side value matching threw and left every option unselected — silently, and only on update. (#2420) ## [1.7.0] - 2026-07-28 From c6623d678b610a7c27512622a79f9810b783cf38 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Wed, 12 Aug 2026 17:19:23 -0600 Subject: [PATCH 03/13] fix(ui): Normalize choice values across all choice inputs Addresses review feedback on #2420. The original fix coerced `selected` in the checkbox/radio update path; this moves the normalization into a shared `shiny/ui/_choices.py` used by the checkbox group, radio buttons, select, selectize, and toolbar select, and fixes the edges the narrower fix left open or introduced: - Widen `ChoicesArg`/`selected` annotations so the documented non-string usage type-checks. `Mapping`'s key type is invariant, so the mapping arm has to be keyed on `Any` -- a narrower union rejects even `dict[str, str]`. - Collapse a sequence `selected` to a scalar for `update_radio_buttons()`, which otherwise throws client-side. `selected=[]` still clears, since an empty array is the one shape that binding special-cases. - Accept any iterable `selected` (`dict_keys`, `set`, generators) instead of stringifying it to an unusable `repr`. - Raise on choice values that collide once stringified, rather than silently dropping an option, and raise a `ValueError` naming `choices` for an empty radio group instead of an `IndexError`. - Resolve `selected` against the actual choice values, so a numerically equal entry (`1.0` vs. `1`) both renders and transmits as the value the option carries. Matching on raw equality instead would restore the server/client disagreement this change exists to remove. - Fix `update_select()`/`update_selectize()`, which had the same defect, and drop the ad-hoc coercion in `_toolbar.py`. - Correct the comment claiming the coercion is a no-op for rendered HTML (`None`/`bool` values change) and the docstring claiming tuples are not JSON-serializable (they are). --- CHANGELOG.md | 8 +- shiny/ui/_choices.py | 197 ++++++++++++++++++++++ shiny/ui/_input_check_radio.py | 114 +++++++------ shiny/ui/_input_select.py | 110 ++++++++++--- shiny/ui/_input_update.py | 77 +++++++-- shiny/ui/_toolbar.py | 17 +- tests/pytest/test_input_check_radio.py | 215 +++++++++++++++++++++++-- 7 files changed, 629 insertions(+), 109 deletions(-) create mode 100644 shiny/ui/_choices.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e6f9e606..006a401c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,7 +49,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * When `@expressify` cannot locate a function's definition, the error now names the function as it appears in the source (rather than a `__name__` a decorator may have rewritten), points at the file and line it looked at, and lists the likely causes — an `async def`, a decorator below `expressify()` that returns a wrapper instead of the original function, or a source file modified after import. (#2016) -* `ui.update_checkbox_group()` and `ui.update_radio_buttons()` now accept non-string `selected` values (e.g. the `int` keys of a `dict[int, str]` passed as `choices`), matching what `ui.input_checkbox_group()` and `ui.input_radio_buttons()` already allowed. Choice values always reach the browser as strings (they become HTML `value` attributes), but the update functions forwarded `selected` unchanged, so the client-side value matching threw and left every option unselected — silently, and only on update. (#2420) +* The choice inputs (`ui.input_checkbox_group()`, `ui.input_radio_buttons()`, `ui.input_select()`, `ui.input_selectize()`, `ui.toolbar_input_select()`) and their `update_*()` counterparts now consistently accept non-string choice values and `selected` values — for example the `int` keys of a `dict[int, str]` passed as `choices`. Choice values become HTML `value` attributes, so the browser only ever sees their string form; the `update_*()` functions forwarded `selected` unchanged, so the client-side value matching failed. For `update_checkbox_group()` this threw *after* clearing every box, leaving nothing selected; for `update_radio_buttons()` and `update_select()` it threw before changing anything (so a bundled label update was silently lost) or deselected everything. The parameter type annotations were widened to match, so these calls now also pass a type checker. (#2413, #2420) + +* `ui.update_radio_buttons(selected=)` now collapses a list or tuple to a single value. A single-selection binding throws on a non-empty array, which aborted the rest of the update — an `selected=[]` still clears the selection, as documented. (#2420) + +* Choice values whose string forms collide (e.g. `choices={0: "zero", "0": "oh"}`) now raise a `ValueError` instead of silently dropping one of the options, and `ui.input_radio_buttons(choices=[])` raises a `ValueError` naming `choices` rather than an `IndexError` from an internal helper. (#2420) + +* `None` and `bool` choice values now render as `value="None"`/`"False"`/`"True"`. Previously htmltools dropped the attribute entirely (or emitted `value=""` for `True`), which made the browser report the default `"on"` for such an option. Note this changes what these inputs report; apps relying on the old `""`/`"on"` values, including saved bookmarks, will read the new strings. (#2420) ## [1.7.0] - 2026-07-28 diff --git a/shiny/ui/_choices.py b/shiny/ui/_choices.py new file mode 100644 index 000000000..6b8d42eee --- /dev/null +++ b/shiny/ui/_choices.py @@ -0,0 +1,197 @@ +""" +Shared normalization for the choice-based inputs (checkbox group, radio buttons, +select, selectize, toolbar select). + +Choice values become HTML ``value`` attributes, so the client only ever sees their +string form. Everything here exists to make the server agree with that: choice values +are coerced to ``str`` once, and ``selected`` is coerced the same way so that the +regenerated options markup and the ``value`` sent in an ``update_*()`` message match +by construction. + +See https://github.com/posit-dev/py-shiny/issues/2272. +""" + +from __future__ import annotations + +from typing import Any, Iterable, Mapping, TypeVar + +# A choice value. +# +# Deliberately `Any` rather than a union of the scalar types we expect. `Mapping`'s key +# type is invariant, so a narrower annotation makes `Mapping[, TagChild]` reject +# `dict[int, str]` -- and, surprisingly, `dict[str, str]` as well. `Any` is also honest +# about the runtime contract: any object is accepted and coerced with `str()`. +ChoiceValue = Any + +_V = TypeVar("_V") + + +def normalize_choices_indexed( + x: Mapping[Any, _V], +) -> tuple[dict[str, _V], dict[str, Any]]: + """ + Coerce choice values (the mapping's keys) to ``str``, preserving order. + + Returns both the stringified mapping and an index from each string form back to the + original choice value, which :func:`resolve_selected_values` needs. + + Raises + ------ + ValueError + If two choice values collide once stringified. Silently dropping one of them + would make an option disappear from the rendered group with no diagnostic. + """ + labels: dict[str, _V] = {} + originals: dict[str, Any] = {} + + for key, label in x.items(): + str_key = str(key) + if str_key in labels: + raise ValueError( + f"Duplicate choice value {str_key!r}: {originals[str_key]!r} and {key!r} " + "are distinct choices but are identical once converted to a string. " + "Choice values become HTML `value` attributes, so they must be unique " + "as strings." + ) + labels[str_key] = label + originals[str_key] = key + + return labels, originals + + +def normalize_choices_mapping(x: Mapping[Any, _V]) -> dict[str, _V]: + """Coerce choice values (the mapping's keys) to ``str``, preserving order.""" + return normalize_choices_indexed(x)[0] + + +def resolve_selected_values(selected: Any, choice_values: Mapping[str, Any]) -> Any: + """ + Replace each ``selected`` entry with the choice value it refers to. + + Usually a no-op, since ``selected`` entries normally *are* choice values. It matters + when an entry compares equal to a choice value without stringifying identically: + ``selected=[1.0]`` against ``choices={1: "a"}`` resolves to ``[1]``, so it both + renders as and transmits ``"1"`` -- the value the option actually carries. + + Without this step, matching on the string form alone would stop honoring a + numerically equal ``selected``, which used to mark the option checked. Matching on + raw equality *instead* is not an option either: that is what left the server marking + an option checked while sending the client a ``"1.0"`` it could never match. + """ + if selected is None or not choice_values: + return selected + + originals = list(choice_values.values()) + + def resolve_one(value: Any) -> Any: + if str(value) in choice_values: + return value + for original in originals: + if original == value: + return original + return value + + if isinstance(selected, (str, bytes)) or not isinstance(selected, Iterable): + return resolve_one(selected) + return [resolve_one(value) for value in selected] + + +def _as_raw_list(x: Any) -> list[Any]: + """ + Coerce ``selected`` to a list of raw (un-stringified) choice values. + + ``str`` and ``bytes`` are iterable but represent a single choice, so they are + treated as scalars. Any other iterable -- including the ``dict_keys``, ``set``, and + generator shapes that a caller may build a selection from -- is consumed as a + sequence. Handling them here keeps them from falling into the scalar branch, where + they would be stringified to their unusable ``repr``. + """ + if x is None: + return [] + elif isinstance(x, (str, bytes)) or not isinstance(x, Iterable): + return [x] + else: + return list(x) + + +def normalize_selected(x: Any) -> str | list[str] | None: + """ + Coerce ``selected`` to string(s), preserving scalar vs. sequence shape. + + Shape matters on the wire: the checkbox group's client-side ``setValue()`` expects + an array while the radio group's expects a scalar, so a scalar ``selected`` must not + grow into a list on its way out. Tuples become lists purely for payload consistency + -- ``json.dumps()`` already serializes a tuple as a JSON array. + """ + if x is None: + return None + elif isinstance(x, str): + return x + elif isinstance(x, bytes) or not isinstance(x, Iterable): + return str(x) + else: + return [str(v) for v in x] + + +def normalize_selected_list(x: Any) -> list[str] | None: + """Coerce ``selected`` to a list of strings, for clients that expect an array.""" + if x is None: + return None + return [str(v) for v in _as_raw_list(x)] + + +def normalize_selected_scalar(x: Any) -> str | None: + """ + Coerce ``selected`` to a single string, for clients that expect a bare scalar. + + A sequence collapses to its first element, and an empty one to ``None``. + """ + if x is None: + return None + values = _as_raw_list(x) + if not values: + return None + return str(values[0]) + + +def normalize_selected_radio(x: Any) -> str | list[str] | None: + """ + Coerce ``selected`` for the radio-button binding, which expects a scalar. + + Sending it a non-empty array is not merely ignored -- ``setValue()`` passes the array + to ``$escape()``, which calls ``.replace()`` on it and throws, aborting the rest of + the message handler (so a bundled label update is lost too). A sequence therefore + collapses to its first element. + + An *empty* sequence is the exception and is preserved as ``[]``: that is the one + array shape ``setValue()`` special-cases, and it clears the selection. Collapsing it + to ``None`` would drop ``value`` from the message entirely and leave the previous + selection in place. + """ + if x is None: + return None + values = _as_raw_list(x) + if not values: + return [] + return str(values[0]) + + +class ChoiceSelection: + """ + Tests whether a choice value is selected: ``choice_value in selection``. + + Comparison is on the string form, since that is the only thing the client can match + against. Entries that refer to a choice value without stringifying identically are + handled by :func:`resolve_selected_values` before they get here, not by loosening the + comparison -- a set lookup keeps this O(1) per choice rather than scanning the + selection for every option in the group. + """ + + def __init__(self, selected: Any) -> None: + self._strings = {str(v) for v in _as_raw_list(selected)} + + def __contains__(self, choice: Any) -> bool: + return str(choice) in self._strings + + def __bool__(self) -> bool: + return bool(self._strings) diff --git a/shiny/ui/_input_check_radio.py b/shiny/ui/_input_check_radio.py index 5f2a8eb53..c3a274ad3 100644 --- a/shiny/ui/_input_check_radio.py +++ b/shiny/ui/_input_check_radio.py @@ -6,27 +6,42 @@ "input_radio_buttons", ) -from typing import Mapping, Optional, Union +from typing import Any, Mapping, Optional, Union from htmltools import Tag, TagAttrs, TagChild, css, div, span, tags from .._docstring import add_example from ..bookmark import restore_input from ..module import resolve_id +from ._choices import ( + ChoiceSelection, + ChoiceValue, + normalize_choices_indexed, + resolve_selected_values, +) from ._html_deps_shinyverse import components_dependencies from ._utils import shiny_input_label -# Canonical format for representing select options. +# Canonical format for representing select options. Choice values are strings here: +# `_normalize_choices()` has already coerced them. _Choices = Mapping[str, TagChild] # Formats available to the user ChoicesArg = Union[ - # ["a", "b", "c"] - "list[str]", - # ("a", "b", "c") - "tuple[str, ...]", - # {"a": "Choice A", "b": tags.i("Choice B")} - _Choices, + # [0, 1, 2] or ["a", "b", "c"] + "list[ChoiceValue]", + # (0, 1, 2) or ("a", "b", "c") + "tuple[ChoiceValue, ...]", + # {"a": "Choice A", 0: tags.i("Choice B")} + Mapping[ChoiceValue, TagChild], +] + +# A single choice value, or several. Coerced with `str()`, so e.g. the `int` keys of a +# `dict[int, str]` passed as `choices` work here too. +SelectedArg = Union[ + ChoiceValue, + "list[ChoiceValue]", + "tuple[ChoiceValue, ...]", ] @@ -177,7 +192,7 @@ def input_checkbox_group( label: TagChild, choices: ChoicesArg, *, - selected: Optional[str | list[str]] = None, + selected: Optional[SelectedArg] = None, inline: bool = False, width: Optional[str] = None, ) -> Tag: @@ -249,7 +264,7 @@ def input_radio_buttons( label: TagChild, choices: ChoicesArg, *, - selected: Optional[str] = None, + selected: Optional[ChoiceValue] = None, inline: bool = False, width: Optional[str] = None, ) -> Tag: @@ -318,34 +333,35 @@ def _generate_options( id: str, type: str, choices: ChoicesArg, - selected: Optional[str | list[str] | tuple[str, ...]], + selected: SelectedArg | None, inline: bool, ) -> Tag: - choicez = _normalize_choices(choices) - selectedz = _normalize_selected(selected) - - selected_list: list[str] - if selectedz is None: - if type == "radio": - selected_list = [list(choicez.keys())[0]] - else: - selected_list = [] - elif isinstance(selectedz, list): - selected_list = selectedz - else: - selected_list = [selectedz] + choicez, choice_values = _normalize_choices_indexed(choices) + + # A radio group must always have something checked, so an omitted `selected` falls + # back to the first choice. Note the check is against `None` specifically: an empty + # `selected` is an explicit request for nothing checked, and must stay that way. + if selected is None and type == "radio": + if not choicez: + raise ValueError( + "`choices` cannot be empty for a radio button group unless " + "`selected` is given." + ) + selected = next(iter(choicez)) + + selection = ChoiceSelection(resolve_selected_values(selected, choice_values)) return div( [ _generate_option( id, type, - value=choice[0], - label=choice[1], - checked=choice[0] in selected_list, + value=value, + label=label, + checked=value in selection, inline=inline, ) - for choice in choicez.items() + for value, label in choicez.items() ], class_="shiny-options-group", ) @@ -381,30 +397,26 @@ def _generate_option( def _normalize_choices(x: ChoicesArg) -> _Choices: - # Choice values end up in an HTML `value` attribute, so they are always strings by - # the time they reach the client. Coerce them here (e.g. the `int` keys of a - # `dict[int, str]`) so that server-side comparisons against `selected` -- and the - # `value` we send in `update_*()` messages -- use the same string form. - if isinstance(x, (list, tuple)): - return {str(k): k for k in x} - else: - return {str(k): v for k, v in x.items()} + return _normalize_choices_indexed(x)[0] -def _normalize_selected( - x: Optional[str | list[str] | tuple[str, ...]], -) -> Optional[str | list[str]]: - """ - Coerce ``selected`` value(s) to string(s), preserving scalar vs. sequence shape. +def _choice_value_index(x: ChoicesArg) -> dict[str, Any]: + """Map each choice value's string form back to the value as the caller wrote it.""" + return _normalize_choices_indexed(x)[1] - ``selected`` values are matched against choice values, which - :func:`_normalize_choices` normalizes to strings, so they must be normalized the - same way. Tuples become lists so the result is JSON-serializable for - ``update_*()`` messages. - """ - if x is None: - return None - elif isinstance(x, (list, tuple)): - return [str(v) for v in x] + +def _normalize_choices_indexed(x: ChoicesArg) -> tuple[_Choices, dict[str, Any]]: + # Coerce choice values to `str` so that the server-side comparison against + # `selected` -- and the `value` we send in `update_*()` messages -- use the same + # string form the client matches on. + # + # This is *not* always a no-op for the rendered HTML, despite choice values landing + # in an HTML `value` attribute either way: htmltools omits an attribute whose value + # is `None` or `False` and renders `True` as `value=""`, so `None`/`bool` choice + # values now render as `value="None"`/`"False"`/`"True"` rather than being dropped. + # That is the intended behavior -- a dropped `value` makes the browser report the + # default `"on"` -- but it does change what such an input reports. + if isinstance(x, (list, tuple)): + return normalize_choices_indexed({k: k for k in x}) else: - return str(x) + return normalize_choices_indexed(x) diff --git a/shiny/ui/_input_select.py b/shiny/ui/_input_select.py index eeb6b0a56..ca5b8859e 100644 --- a/shiny/ui/_input_select.py +++ b/shiny/ui/_input_select.py @@ -18,25 +18,41 @@ from .._docstring import add_example from ..bookmark import restore_input from ..module import resolve_id +from ._choices import ( + ChoiceSelection, + ChoiceValue, + normalize_choices_indexed, + normalize_choices_mapping, + resolve_selected_values, +) from ._html_deps_external import selectize_deps from ._utils import JSEval, extract_js_keys, shiny_input_label +# Canonical format for representing select options. Choice values are strings here: +# `_normalize_choices()` has already coerced them. _Choices = Mapping[str, str] _OptGrpChoices = Mapping[str, _Choices] -# Canonical format for representing select options. _SelectChoices = Union[_Choices, _OptGrpChoices] -# Formats available to the user +# Formats available to the user. Choice values are coerced with `str()`, so e.g. the +# `int` keys of a `dict[int, str]` are supported. SelectChoicesArg = Union[ - # ["a", "b", "c"] - "list[str]", - # ("a", "b", "c") - "tuple[str, ...]", - # {"a": "Choice A", "b": tags.i("Choice B")} - _Choices, - # optgroup {"Group A": {"a1": "Choice A1", "a2": tags.i("Choice A2")}, "Group B": {}} - _OptGrpChoices, + # [0, 1, 2] or ["a", "b", "c"] + "list[ChoiceValue]", + # (0, 1, 2) or ("a", "b", "c") + "tuple[ChoiceValue, ...]", + # {"a": "Choice A", 0: "Choice B"} + Mapping[ChoiceValue, str], + # optgroup {"Group A": {"a1": "Choice A1", "a2": "Choice A2"}, "Group B": {}} + Mapping[ChoiceValue, Mapping[ChoiceValue, str]], +] + +# A single choice value, or several. +SelectSelectedArg = Union[ + ChoiceValue, + "list[ChoiceValue]", + "tuple[ChoiceValue, ...]", ] @@ -54,7 +70,7 @@ def input_selectize( label: TagChild, choices: SelectChoicesArg, *, - selected: Optional[str | list[str]] = None, + selected: Optional[SelectSelectedArg] = None, multiple: bool = False, width: Optional[str] = None, remove_button: Optional[bool] = None, @@ -134,7 +150,7 @@ def input_select( label: TagChild, choices: SelectChoicesArg, *, - selected: Optional[str | list[str]] = None, + selected: Optional[SelectSelectedArg] = None, multiple: bool = False, selectize: bool | MISSING_TYPE = DEPRECATED, width: Optional[str] = None, @@ -239,7 +255,7 @@ def _input_select_impl( label: TagChild, choices: SelectChoicesArg, *, - selected: Optional[str | list[str]] = None, + selected: Optional[SelectSelectedArg] = None, multiple: bool = False, selectize: bool = False, width: Optional[str] = None, @@ -259,6 +275,8 @@ def _input_select_impl( selected = restore_input(resolved_id, selected) if selected is None and not multiple: selected = _find_first_option(choices_) + else: + selected = resolve_selected_values(selected, _choice_value_index(choices)) if options is None: options = {} @@ -331,12 +349,49 @@ def _update_options( def _normalize_choices(x: SelectChoicesArg) -> _SelectChoices: + # Coerce choice values to `str` so that the option `value` attributes we render and + # the `value` we send in `update_*()` messages agree. Optgroup labels and the choice + # values nested inside them get the same treatment. + # https://github.com/posit-dev/py-shiny/issues/2272 if x is None: raise TypeError("`choices` must be a list, tuple, or dict.") elif isinstance(x, (list, tuple)): - return {k: k for k in x} - else: - return x + return normalize_choices_mapping({k: k for k in x}) + + # `_SelectChoices` is "all flat" or "all optgroup", with no arm for a per-key mix, so + # the comprehension's element type does not fit either. The runtime shape is whatever + # the caller passed, which `_render_choices()` handles key by key. + normalized = normalize_choices_mapping(x) + result: dict[str, Any] = { + key: ( + normalize_choices_mapping(cast("Mapping[Any, str]", value)) + if isinstance(value, Mapping) + else value + ) + for key, value in normalized.items() + } + return cast(_SelectChoices, result) + + +def _choice_value_index(x: SelectChoicesArg) -> dict[str, Any]: + """ + Map each choice value's string form back to the value as the caller wrote it. + + Optgroup labels are not choice values, so only the options nested inside a group are + indexed -- never the group key itself. + """ + if x is None: + return {} + elif isinstance(x, (list, tuple)): + return normalize_choices_indexed({k: k for k in x})[1] + + index: dict[str, Any] = {} + for key, value in x.items(): + if isinstance(value, Mapping): + index.update(normalize_choices_indexed(cast("Mapping[Any, str]", value))[1]) + else: + index[str(key)] = key + return index def _contains_html(x: _SelectChoices) -> bool: @@ -353,7 +408,13 @@ def _contains_html(x: _SelectChoices) -> bool: def _render_choices( - x: _SelectChoices, selected: Optional[str | list[str]] = None + x: _SelectChoices, selected: Optional[SelectSelectedArg] = None +) -> TagList: + return _render_choices_with_selection(x, ChoiceSelection(selected)) + + +def _render_choices_with_selection( + x: _SelectChoices, selection: ChoiceSelection ) -> TagList: result = TagList() @@ -364,17 +425,16 @@ def _render_choices( if isinstance(v, Mapping): result.append( tags.optgroup( - *(_render_choices(cast(_SelectChoices, v), selected)), label=k + *( + _render_choices_with_selection( + cast(_SelectChoices, v), selection + ) + ), + label=k, ) ) else: - is_selected = False - if isinstance(selected, list): - is_selected = k in selected - else: - is_selected = k == selected - - result.append(tags.option(v, value=k, selected=is_selected)) + result.append(tags.option(v, value=k, selected=k in selection)) return result diff --git a/shiny/ui/_input_update.py b/shiny/ui/_input_update.py index 6f835ea3e..49f07354c 100644 --- a/shiny/ui/_input_update.py +++ b/shiny/ui/_input_update.py @@ -36,9 +36,28 @@ from ..module import ResolvedId, resolve_id from ..session import require_active_session, session_context from ..types import ActionButtonValue -from ._input_check_radio import ChoicesArg, _generate_options, _normalize_selected +from ._choices import ( + normalize_selected, + normalize_selected_list, + normalize_selected_radio, + resolve_selected_values, +) +from ._input_check_radio import ( + ChoicesArg, + SelectedArg, + _choice_value_index, + _generate_options, +) from ._input_date import _as_date_attr -from ._input_select import SelectChoicesArg, _normalize_choices, _render_choices +from ._input_select import ( + SelectChoicesArg, + SelectSelectedArg, +) +from ._input_select import _choice_value_index as _select_choice_value_index +from ._input_select import ( + _normalize_choices, + _render_choices, +) from ._input_slider import SliderStepArg, SliderValueArg, _as_numeric, _slider_type from ._utils import JSEval, _session_on_flush_send_msg, extract_js_keys @@ -313,7 +332,7 @@ def update_checkbox_group( *, label: Optional[TagChild] = None, choices: Optional[ChoicesArg] = None, - selected: Optional[str | list[str] | tuple[str, ...]] = None, + selected: Optional[SelectedArg] = None, inline: bool = False, session: Optional[Session] = None, ) -> None: @@ -365,7 +384,7 @@ def update_radio_buttons( *, label: Optional[TagChild] = None, choices: Optional[ChoicesArg] = None, - selected: Optional[str] = None, + selected: Optional[SelectedArg] = None, inline: bool = False, session: Optional[Session] = None, ) -> None: @@ -416,7 +435,7 @@ def _update_choice_input( type: Literal["checkbox", "radio"], label: Optional[TagChild] = None, choices: Optional[ChoicesArg] = None, - selected: Optional[str | list[str] | tuple[str, ...]] = None, + selected: Optional[SelectedArg] = None, inline: bool = False, session: Optional[Session] = None, ) -> None: @@ -426,7 +445,22 @@ def _update_choice_input( # which are always strings. Normalize the same way `input_checkbox_group()` / # `input_radio_buttons()` do so that, e.g., the `int` keys of a `dict[int, str]` # work here too. https://github.com/posit-dev/py-shiny/issues/2272 - selected_values = _normalize_selected(selected) + # + # When `choices` is given, resolve against it first, so that a `selected` entry which + # merely compares equal to a choice value (`1.0` vs. `1`) is sent as the string that + # choice actually renders. Normalizing once here and reusing the result for both the + # options markup and the message keeps the two in agreement by construction. + if choices is not None: + selected = resolve_selected_values(selected, _choice_value_index(choices)) + + # Shape matters as much as type. A radio group's client-side `setValue()` passes a + # non-empty array straight to `$escape()`, which calls `.replace()` on it and throws + # -- aborting the handler before it applies the label update or fires `change`. So a + # sequence has to collapse to a scalar here, while a checkbox group keeps its array. + if type == "radio": + selected_values = normalize_selected_radio(selected) + else: + selected_values = normalize_selected(selected) options = None if choices is not None: @@ -438,6 +472,11 @@ def _update_choice_input( id=resolved_id, type=type, choices=choices, + # `_generate_options()` normalizes again, because the `input_*()` + # constructors call it with a raw `selected`. That is a no-op on the value + # built above -- resolution maps a string that is already a choice value to + # itself, and `str()` is idempotent -- which is what lets the options markup + # and the message's `value` below come from one normalization. selected=selected_values, inline=inline, ) @@ -663,7 +702,7 @@ def update_select( *, label: Optional[TagChild] = None, choices: Optional[SelectChoicesArg] = None, - selected: Optional[str | list[str]] = None, + selected: Optional[SelectSelectedArg] = None, session: Optional[Session] = None, ) -> None: """ @@ -698,9 +737,16 @@ def update_select( session = require_active_session(session) - selected_values = selected - if isinstance(selected, str): - selected_values = [selected] + # ``'s `setValue()` does `$(el).val(value)`, and jQuery matches option values - # with a strict `===`, so a non-string `selected` silently deselects everything. - # Normalize to strings, matching the option `value` attributes `_render_choices()` - # emits. https://github.com/posit-dev/py-shiny/issues/2272 + # jQuery matches option values with strict `===`, so a non-string `selected` + # silently deselects everything. Resolve to the string form the options render. if choices is not None: selected = resolve_selected_values( selected, _select_choice_value_index(choices) From a84a2a304106b41296be84a495de4f96470b0717 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Tue, 18 Aug 2026 11:24:45 -0600 Subject: [PATCH 10/13] refactor(ui): Return the canonical choice value from resolve_selected_values Addresses review feedback: the fast path returned the selected entry unchanged when its string form matched a choice, which contradicted the docstring. Behavior is identical downstream (all consumers stringify), but returning the canonical value is consistent and handles objects whose __str__ collides with a key without comparing equal to it. --- shiny/ui/_choices.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shiny/ui/_choices.py b/shiny/ui/_choices.py index cf69a8c67..97772b21b 100644 --- a/shiny/ui/_choices.py +++ b/shiny/ui/_choices.py @@ -81,7 +81,7 @@ def resolve_selected_values(selected: Any, choice_values: Mapping[str, Any]) -> def resolve_one(value: Any) -> Any: if str(value) in choice_values: - return value + return choice_values[str(value)] for original in originals: if original == value: return original From 4436a04dcf9d17f68e7df85b85d0e449256cbfae Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Wed, 19 Aug 2026 11:27:09 -0600 Subject: [PATCH 11/13] fix(ui): Match a `selected` value to a choice by its string form Addresses @cpsievert's review on #2420. `resolve_selected()` (was `resolve_selected_values()`) now matches on the string form only, since that is all the client ever sees. The general `==` fallback it replaces made `selected=True` name a choice value of `1`, and could raise or return a non-`bool` for array-like and custom types. An integral `float` still matches the integer it names, so `selected=1.0` against `choices={1: "a"}` keeps working, as it does in R Shiny where `as.character(1.0)` is `"1"`. Resolution now yields the choice's string form rather than the choice value as the caller wrote it, so the string-to-original index is gone (`normalize_choices_indexed()`, `_choice_value_index()`). Every consumer stringified the result anyway. Dropping it also removes the silent last-one-wins overwrite when two optgroups held the same choice value; the markup for that case is unchanged and matches R Shiny, which permits duplicate values. `ChoiceValue` is now `str | int | float | bool | None` instead of `Any`, so the public annotations no longer accept an arbitrary object. Sequence arms use `Sequence[ChoiceValue]` (invariance rules out `list[ChoiceValue]`, which would reject `list[str]`), and `Mapping` keys stay `Any` because an exact key type rejects `dict[int, str]`. A bare `str` satisfies `Sequence[str]`, so `choices` that is a string, or is not iterable, now raises a clear `TypeError` instead of an `AttributeError` from a private helper. A select input's top-level `choices` maps optgroup labels alongside choice values, so its duplicate-key error now names both. The check has to stay: the normalized form is a `dict` keyed by the string, so a collision there would drop an entry with no diagnostic. Also drops the redundant underscore prefixes inside the private `_choices` module (`_V`, `_as_raw_list`), and removes `_contains_html()`, dead since #2049 removed the deprecation warning that was its only caller. --- CHANGELOG.md | 4 +- shiny/ui/_choices.py | 132 ++++++++++++++----------- shiny/ui/_input_check_radio.py | 52 ++++------ shiny/ui/_input_select.py | 70 ++++++------- shiny/ui/_input_update.py | 35 +++---- shiny/ui/_toolbar.py | 8 +- tests/pytest/test_input_check_radio.py | 46 ++++++++- 7 files changed, 191 insertions(+), 156 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38296b172..6a7eb3069 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,7 +61,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `ui.update_radio_buttons(choices=[])` now clears the set of choices. This was the documented behavior, but didn't actually work and raised an `IndexError` instead. `ui.input_radio_buttons(choices=[])` still raises, but now with a more helpful `ValueError` that names `choices`. (#2420) -* Choice values whose string forms collide, such as `choices=[0, "0"]`, now raise a `ValueError`. Previously both options rendered with the same underlying value, so the input could not report which one the user chose. (#2420) +* Choice values whose string forms collide, such as `choices=[0, "0"]`, now raise a `ValueError`. Previously both options rendered with the same underlying value, so the input could not report which one the user chose. In a select input's `choices`, optgroup labels share the top-level mapping with choice values, so a label and a value must not collide either. Two separate optgroups may still hold the same choice value. (#2420) + +* A `selected` value now names a choice by its string form. The input constructors previously compared raw Python values, so `ui.input_radio_buttons(choices={1: "one"}, selected=True)` checked the `1` option; it no longer does, which matches R Shiny. An integral `float` still names the integer it equals, so `selected=1.0` continues to match a choice value of `1`. (#2420) * `None` and `bool` choice values now render as `value="None"`, `value="False"`, and `value="True"`. Previously the browser reported `"on"` or `""` for these options, so `None` and `False` were indistinguishable and neither could be updated. Apps and saved bookmarks that hold the old `""`/`"on"` values will read the new strings. (#2420) diff --git a/shiny/ui/_choices.py b/shiny/ui/_choices.py index 97772b21b..59b694e16 100644 --- a/shiny/ui/_choices.py +++ b/shiny/ui/_choices.py @@ -14,85 +14,101 @@ from __future__ import annotations -from collections.abc import Iterable, Mapping -from typing import Any, TypeVar, cast - -# A choice value. -# -# Deliberately `Any` rather than a union of the scalar types we expect. `Mapping`'s key -# type is invariant, so a narrower annotation makes `Mapping[, TagChild]` reject -# `dict[int, str]` or `dict[str, str]`. -ChoiceValue = Any - -_V = TypeVar("_V") - - -def normalize_choices_indexed( - x: Mapping[Any, _V], -) -> tuple[dict[str, _V], dict[str, Any]]: +from collections.abc import Collection, Iterable, Mapping +from typing import Any, Literal, TypeVar, Union, cast + +# A choice value. The client only ever sees ``str(value)``, so these are the types with +# an unambiguous string form -- the ones the choice inputs document and test. +ChoiceValue = Union[str, int, float, bool, None] + +# A choice value in a `Mapping` key position, which has to stay `Any`: `Mapping`'s key +# type is invariant, so `Mapping[ChoiceValue, TagChild]` rejects `dict[int, str]` -- and +# even `dict[str, str]`. The runtime contract is the same as `ChoiceValue`. +ChoiceKey = Any + +# What a normalized mapping's keys hold. A select input's `choices` maps optgroup labels +# alongside choice values, so its top level holds both. +ChoiceKeyKind = Literal["choice value", "choice value or optgroup label"] + +_DUPLICATE_KEY_REASONS: dict[ChoiceKeyKind, str] = { + "choice value": ( + "Choice values become HTML `value` attributes, so they must be unique as " + "strings." + ), + "choice value or optgroup label": ( + "A select input's `choices` holds choice values and optgroup labels in one " + "mapping, so both must be unique as strings." + ), +} + +V = TypeVar("V") + + +def normalize_choices_mapping( + x: Mapping[Any, V], *, keys_are: ChoiceKeyKind = "choice value" +) -> dict[str, V]: """ Coerce choice values (the mapping's keys) to ``str``, preserving order. - Returns both the stringified mapping and an index from each string form back to the - original choice value, which :func:`resolve_selected_values` needs. - Raises ------ ValueError - If two choice values collide once stringified. Silently dropping one of them - would make an option disappear from the rendered group with no diagnostic. + If two keys collide once stringified. The normalized form is a ``dict`` keyed by + the string form, so one of the two entries would otherwise disappear from the + rendered input with no diagnostic. """ - labels: dict[str, _V] = {} + normalized: dict[str, V] = {} originals: dict[str, Any] = {} for key, label in x.items(): str_key = str(key) - if str_key in labels: + if str_key in normalized: raise ValueError( - f"Duplicate choice value {str_key!r}: {originals[str_key]!r} and {key!r} " - "are distinct choices but are identical once converted to a string. " - "Choice values become HTML `value` attributes, so they must be unique " - "as strings." + f"Duplicate {keys_are} {str_key!r}: {originals[str_key]!r} and {key!r} " + f"are distinct but are identical once converted to a string. " + + _DUPLICATE_KEY_REASONS[keys_are] ) - labels[str_key] = label + normalized[str_key] = label originals[str_key] = key - return labels, originals + return normalized -def normalize_choices_mapping(x: Mapping[Any, _V]) -> dict[str, _V]: - """Coerce choice values (the mapping's keys) to ``str``, preserving order.""" - return normalize_choices_indexed(x)[0] - - -def resolve_selected_values(selected: Any, choice_values: Mapping[str, Any]) -> Any: +def resolve_selected(selected: Any, choice_values: Collection[str]) -> Any: """ - Replace each ``selected`` entry with the choice value it refers to. + Replace each ``selected`` entry with the string form of the choice value it names. - Usually a no-op, since ``selected`` entries normally *are* choice values. It matters - when an entry compares equal to a choice value without stringifying identically: - ``selected=[1.0]`` against ``choices={1: "a"}`` resolves to ``[1]``, so it both - renders as and transmits ``"1"`` -- the value the option actually carries. - """ - if selected is None or not choice_values: - return selected + Matching is on the string form, since that is the only thing the client can match + against, so a ``selected`` of ``True`` does not name the choice value ``1``. An + integral ``float`` is the one exception: it also matches the integer it equals, so + ``selected=[1.0]`` against ``choices={1: "a"}`` resolves to ``"1"``, the value the + option actually carries. (R Shiny gets that case for free, since + ``as.character(1.0)`` is ``"1"``.) - originals = list(choice_values.values()) + An entry that names no choice passes through as its own string form, so an + ``update_*()`` message still carries what the caller asked for. + """ + if selected is None: + return None - def resolve_one(value: Any) -> Any: - if str(value) in choice_values: - return choice_values[str(value)] - for original in originals: - if original == value: - return original - return value + def resolve_one(value: Any) -> str: + as_str = str(value) + if as_str in choice_values: + return as_str + # `str(1.0)` is `"1.0"`, so an integral float misses the `1` it names. `bool` is + # a subclass of `int` rather than `float`, so `True` stays `"True"` here. + if isinstance(value, float) and value.is_integer(): + as_int = str(int(value)) + if as_int in choice_values: + return as_int + return as_str if isinstance(selected, (str, bytes)) or not isinstance(selected, Iterable): return resolve_one(selected) return [resolve_one(value) for value in cast("Iterable[Any]", selected)] -def _as_raw_list(x: Any) -> list[Any]: +def as_raw_list(x: Any) -> list[Any]: """ Coerce ``selected`` to a list of raw (un-stringified) choice values. @@ -133,7 +149,7 @@ def normalize_selected_list(x: Any) -> list[str] | None: """Coerce ``selected`` to a list of strings, for clients that expect an array.""" if x is None: return None - return [str(v) for v in _as_raw_list(x)] + return [str(v) for v in as_raw_list(x)] def normalize_selected_scalar(x: Any) -> str | None: @@ -144,7 +160,7 @@ def normalize_selected_scalar(x: Any) -> str | None: """ if x is None: return None - values = _as_raw_list(x) + values = as_raw_list(x) if not values: return None return str(values[0]) @@ -166,7 +182,7 @@ def normalize_selected_radio(x: Any) -> str | list[str] | None: """ if x is None: return None - values = _as_raw_list(x) + values = as_raw_list(x) if not values: return [] if len(values) > 1: @@ -183,12 +199,12 @@ class ChoiceSelection: Tests whether a choice value is selected: ``choice_value in selection``. Comparison is on the string form, since that is the only thing the client can match - against. Entries that refer to a choice value without stringifying identically are - handled by :func:`resolve_selected_values` before they get here. + against. Entries that name a choice value without stringifying identically are + handled by :func:`resolve_selected` before they get here. """ def __init__(self, selected: Any) -> None: - self._strings = {str(v) for v in _as_raw_list(selected)} + self._strings = {str(v) for v in as_raw_list(selected)} def __contains__(self, choice: Any) -> bool: return str(choice) in self._strings diff --git a/shiny/ui/_input_check_radio.py b/shiny/ui/_input_check_radio.py index d61ea82ac..981fcaee1 100644 --- a/shiny/ui/_input_check_radio.py +++ b/shiny/ui/_input_check_radio.py @@ -6,7 +6,7 @@ "input_radio_buttons", ) -from typing import Any, Mapping, Optional, Union +from typing import Iterable, Mapping, Optional, Sequence, Union from htmltools import Tag, TagAttrs, TagChild, css, div, span, tags @@ -14,34 +14,29 @@ from ..bookmark import restore_input from ..module import resolve_id from ._choices import ( + ChoiceKey, ChoiceSelection, ChoiceValue, - normalize_choices_indexed, - resolve_selected_values, + normalize_choices_mapping, + resolve_selected, ) from ._html_deps_shinyverse import components_dependencies from ._utils import shiny_input_label -# Canonical format for representing select options. Choice values are strings here: -# `_normalize_choices()` has already coerced them. -_Choices = Mapping[str, TagChild] - -# Formats available to the user +# Formats available to the user. Choice values are coerced with `str()`, so e.g. the +# `int` keys of a `dict[int, str]` are supported. ChoicesArg = Union[ - # [0, 1, 2] or ["a", "b", "c"] - "list[ChoiceValue]", - # (0, 1, 2) or ("a", "b", "c") - "tuple[ChoiceValue, ...]", + # [0, 1, 2] or ("a", "b", "c") + Sequence[ChoiceValue], # {"a": "Choice A", 0: tags.i("Choice B")} - Mapping[ChoiceValue, TagChild], + Mapping[ChoiceKey, TagChild], ] # A single choice value, or several. Coerced with `str()`, so e.g. the `int` keys of a # `dict[int, str]` passed as `choices` work here too. SelectedArg = Union[ ChoiceValue, - "list[ChoiceValue]", - "tuple[ChoiceValue, ...]", + Sequence[ChoiceValue], ] @@ -264,7 +259,7 @@ def input_radio_buttons( label: TagChild, choices: ChoicesArg, *, - selected: Optional[ChoiceValue] = None, + selected: Optional[SelectedArg] = None, inline: bool = False, width: Optional[str] = None, ) -> Tag: @@ -336,7 +331,7 @@ def _generate_options( selected: Optional[SelectedArg], inline: bool, ) -> Tag: - choicez, choice_values = _normalize_choices_indexed(choices) + choicez = _normalize_choices(choices) # A radio group must always have something checked, so an omitted `selected` falls # back to the first choice. Note the check is against `None` specifically: an empty @@ -349,7 +344,7 @@ def _generate_options( ) selected = next(iter(choicez)) - selection = ChoiceSelection(resolve_selected_values(selected, choice_values)) + selection = ChoiceSelection(resolve_selected(selected, choicez.keys())) return div( [ @@ -396,21 +391,16 @@ def _generate_option( ) -def _normalize_choices(x: ChoicesArg) -> _Choices: - return _normalize_choices_indexed(x)[0] - - -def _choice_value_index(x: ChoicesArg) -> dict[str, Any]: - """Map each choice value's string form back to the value as the caller wrote it.""" - return _normalize_choices_indexed(x)[1] - - -def _normalize_choices_indexed(x: ChoicesArg) -> tuple[_Choices, dict[str, Any]]: +def _normalize_choices(x: ChoicesArg) -> dict[str, TagChild]: """ Normalize choices, coercing choice values to `str` so they match the string form the client reports. """ - if isinstance(x, (list, tuple)): - return normalize_choices_indexed({k: k for k in x}) + if isinstance(x, Mapping): + return normalize_choices_mapping(x) + elif isinstance(x, Iterable) and not isinstance(x, (str, bytes)): + return normalize_choices_mapping({k: k for k in x}) else: - return normalize_choices_indexed(x) + # A bare `str` satisfies `Sequence[ChoiceValue]` statically, but iterating it + # would turn each character into its own choice. + raise TypeError("`choices` must be a list, tuple, or dict.") diff --git a/shiny/ui/_input_select.py b/shiny/ui/_input_select.py index 73abb12bd..d62314642 100644 --- a/shiny/ui/_input_select.py +++ b/shiny/ui/_input_select.py @@ -11,7 +11,7 @@ ) import copy from json import dumps -from typing import Any, Mapping, Optional, Union, cast +from typing import Any, Iterable, Mapping, Optional, Sequence, Union, cast from htmltools import Tag, TagAttrs, TagChild, TagList, css, div, tags @@ -19,11 +19,11 @@ from ..bookmark import restore_input from ..module import resolve_id from ._choices import ( + ChoiceKey, ChoiceSelection, ChoiceValue, - normalize_choices_indexed, normalize_choices_mapping, - resolve_selected_values, + resolve_selected, ) from ._html_deps_external import selectize_deps from ._utils import JSEval, extract_js_keys, shiny_input_label @@ -38,21 +38,18 @@ # Formats available to the user. Choice values are coerced with `str()`, so e.g. the # `int` keys of a `dict[int, str]` are supported. SelectChoicesArg = Union[ - # [0, 1, 2] or ["a", "b", "c"] - "list[ChoiceValue]", - # (0, 1, 2) or ("a", "b", "c") - "tuple[ChoiceValue, ...]", + # [0, 1, 2] or ("a", "b", "c") + Sequence[ChoiceValue], # {"a": "Choice A", 0: "Choice B"} - Mapping[ChoiceValue, str], + Mapping[ChoiceKey, str], # optgroup {"Group A": {"a1": "Choice A1", "a2": "Choice A2"}, "Group B": {}} - Mapping[ChoiceValue, Mapping[ChoiceValue, str]], + Mapping[ChoiceKey, Mapping[ChoiceKey, str]], ] # A single choice value, or several. SelectSelectedArg = Union[ ChoiceValue, - "list[ChoiceValue]", - "tuple[ChoiceValue, ...]", + Sequence[ChoiceValue], ] @@ -276,7 +273,7 @@ def _input_select_impl( if selected is None and not multiple: selected = _find_first_option(choices_) else: - selected = resolve_selected_values(selected, _choice_value_index(choices)) + selected = resolve_selected(selected, _choice_value_strings(choices_)) if options is None: options = {} @@ -356,16 +353,23 @@ def _normalize_choices(x: SelectChoicesArg) -> _SelectChoices: See https://github.com/posit-dev/py-shiny/issues/2272. """ - if x is None: + if isinstance(x, Iterable) and not isinstance(x, (str, bytes, Mapping)): + # A sequence's entries are both the choice value and the label, so the label is + # the value's string form. + return normalize_choices_mapping({k: str(k) for k in x}) + elif not isinstance(x, Mapping): + # A bare `str` satisfies `Sequence[ChoiceValue]` statically, but iterating it + # would turn each character into its own choice. raise TypeError("`choices` must be a list, tuple, or dict.") - elif isinstance(x, (list, tuple)): - return normalize_choices_mapping({k: k for k in x}) + + # The top-level mapping holds optgroup labels alongside choice values, so a + # collision there is reported as either one. + normalized = normalize_choices_mapping(x, keys_are="choice value or optgroup label") # The result may mix flat options and optgroups at the top level (e.g. # `{"a": "A", "Group B": {...}}`). That matches neither arm of the # `_SelectChoices` union, hence the `cast`, but `_render_choices()` # checks each value with `isinstance`, so the mix is fine at runtime. - normalized = normalize_choices_mapping(x) result: dict[str, Any] = { key: (normalize_choices_mapping(value) if isinstance(value, Mapping) else value) for key, value in normalized.items() @@ -373,38 +377,22 @@ def _normalize_choices(x: SelectChoicesArg) -> _SelectChoices: return cast(_SelectChoices, result) -def _choice_value_index(x: SelectChoicesArg) -> dict[str, Any]: +def _choice_value_strings(x: _SelectChoices) -> set[str]: """ - Map each choice value's string form back to the value as the caller wrote it. + Collect the choice values of already-normalized choices, for `resolve_selected()`. Optgroup labels are not choice values, so only the options nested inside a group are - indexed (never the group key itself). + collected (never the group key itself). Two groups may hold the same choice value: + duplicates are legal HTML, and the browser reports only the value, so the options are + indistinguishable either way. """ - if x is None: - return {} - elif isinstance(x, (list, tuple)): - return normalize_choices_indexed({k: k for k in x})[1] - - index: dict[str, Any] = {} + values: set[str] = set() for key, value in x.items(): if isinstance(value, Mapping): - index.update(normalize_choices_indexed(value)[1]) - else: - index[str(key)] = key - return index - - -def _contains_html(x: _SelectChoices) -> bool: - for v in x.values(): - if isinstance(v, Mapping): - # Check the `_Choices` values of `_OptGrpChoices` - for vv in v.values(): - if not isinstance(vv, str): - return True + values.update(value.keys()) else: - if not isinstance(v, str): - return True - return False + values.add(key) + return values def _render_choices( diff --git a/shiny/ui/_input_update.py b/shiny/ui/_input_update.py index b3ef4309a..80beeb6ad 100644 --- a/shiny/ui/_input_update.py +++ b/shiny/ui/_input_update.py @@ -40,20 +40,20 @@ normalize_selected, normalize_selected_list, normalize_selected_radio, - resolve_selected_values, + resolve_selected, ) from ._input_check_radio import ( ChoicesArg, SelectedArg, - _choice_value_index, _generate_options, ) +from ._input_check_radio import _normalize_choices as _check_radio_normalize_choices from ._input_date import _as_date_attr from ._input_select import ( SelectChoicesArg, SelectSelectedArg, ) -from ._input_select import _choice_value_index as _select_choice_value_index +from ._input_select import _choice_value_strings as _select_choice_value_strings from ._input_select import ( _normalize_choices, _render_choices, @@ -444,7 +444,9 @@ def _update_choice_input( # The client matches `value` against the options' HTML `value` attributes, which are # always strings. Resolves `selected` against the choices (see issue 2272) if choices is not None: - selected = resolve_selected_values(selected, _choice_value_index(choices)) + selected = resolve_selected( + selected, _check_radio_normalize_choices(choices).keys() + ) # A radio group's client-side `setValue()` throws on a non-empty array, so collapse # to a scalar; a checkbox group keeps its array. @@ -732,21 +734,18 @@ def update_select( session = require_active_session(session) - # jQuery matches option values with strict `===`, so a non-string `selected` - # silently deselects everything. Resolve to the string form the options render. - if choices is not None: - selected = resolve_selected_values( - selected, _select_choice_value_index(choices) - ) - - selected_values = normalize_selected_list(selected) - if choices is None: options = None else: - option_tags = _render_choices(_normalize_choices(choices), selected) + # jQuery matches option values with strict `===`, so a non-string `selected` + # silently deselects everything. Resolve to the string form the options render. + choices_ = _normalize_choices(choices) + selected = resolve_selected(selected, _select_choice_value_strings(choices_)) + option_tags = _render_choices(choices_, selected) options = session._process_ui(option_tags)["html"] + selected_values = normalize_selected_list(selected) + msg = { "label": session._process_ui(label) if label is not None else None, "options": options, @@ -828,7 +827,8 @@ def update_selectize( # [{"label": "Foo", "value": "foo", "optgroup": "foo"}, ...] flat_choices: list[FlatSelectChoice] = [] if choices is not None: - for k, v in _normalize_choices(choices).items(): + choices_ = _normalize_choices(choices) + for k, v in choices_.items(): if not isinstance(v, Mapping): flat_choices.append(FlatSelectChoice(value=k, label=v)) else: # The optgroup case @@ -839,10 +839,7 @@ def update_selectize( ] ) - if choices is not None: - selected = resolve_selected_values( - selected, _select_choice_value_index(choices) - ) + selected = resolve_selected(selected, _select_choice_value_strings(choices_)) selected_values = normalize_selected_list(selected) diff --git a/shiny/ui/_toolbar.py b/shiny/ui/_toolbar.py index 17c89a2ff..7e29e0356 100644 --- a/shiny/ui/_toolbar.py +++ b/shiny/ui/_toolbar.py @@ -24,12 +24,12 @@ from ._choices import ( ChoiceValue, normalize_selected_scalar, - resolve_selected_values, + resolve_selected, ) from ._html_deps_shinyverse import components_dependencies from ._input_select import ( SelectChoicesArg, - _choice_value_index, + _choice_value_strings, _find_first_option, _normalize_choices, _render_choices, @@ -540,7 +540,7 @@ def toolbar_input_select( if selected is None: selected = _find_first_option(choices_normalized) else: - selected = resolve_selected_values(selected, _choice_value_index(choices)) + selected = resolve_selected(selected, _choice_value_strings(choices_normalized)) # Select element gets its own ID for label association select_id = f"{resolved_id}-select" @@ -677,8 +677,8 @@ def update_toolbar_input_select( options_processed = None if choices is not None: - selected = resolve_selected_values(selected, _choice_value_index(choices)) choices_normalized = _normalize_choices(choices) + selected = resolve_selected(selected, _choice_value_strings(choices_normalized)) options_html = _render_choices(choices_normalized, selected) options_processed = str(options_html) diff --git a/tests/pytest/test_input_check_radio.py b/tests/pytest/test_input_check_radio.py index 73db39d5b..a0c536c03 100644 --- a/tests/pytest/test_input_check_radio.py +++ b/tests/pytest/test_input_check_radio.py @@ -105,8 +105,9 @@ def test_input_checkbox_group_mixed_value_types( def test_input_checkbox_group_numerically_equal_selected(): - # `1.0` is not `str(1)`, but it *is* `== 1`, so it resolves to the choice value `1` - # and renders as the `"1"` the client will match on. + # `str(1.0)` is `"1.0"`, so an integral float is matched against the integer it + # names as well, and renders as the `"1"` the client will match on. R Shiny gets + # this case for free, since `as.character(1.0)` is `"1"`. html = ui.input_checkbox_group( "x", label="L", choices={1: "a", 2: "b"}, selected=[1.0] ).get_html_string() @@ -114,6 +115,28 @@ def test_input_checkbox_group_numerically_equal_selected(): assert 'value="1" checked="checked"' in html +def test_bool_selected_does_not_name_an_int_choice( + session: _MessageCapturingSession, +): + # Matching is on the string form, so `True` does not name the choice value `1` even + # though `True == 1`. This matches R Shiny, where `"TRUE"` and `"1"` differ. + html = ui.input_radio_buttons( + "x", label="L", choices={1: "one"}, selected=True + ).get_html_string() + assert _checked_count(html) == 0 + + msg = _send(session, ui.update_radio_buttons, choices={1: "one"}, selected=True) + assert msg["value"] == "True" + + +def test_choices_must_not_be_a_bare_string(): + # A `str` is a `Sequence[str]`, so it satisfies the `choices` annotation; iterating + # it would silently turn each character into its own choice. + for input_fn in (ui.input_checkbox_group, ui.input_radio_buttons, ui.input_select): + with pytest.raises(TypeError, match="must be a list, tuple, or dict"): + input_fn("x", label="L", choices=cast(Any, "abc")) + + def test_choice_values_are_rendered_for_falsy_types(): # htmltools omits an attribute whose value is `None`/`False` and renders `True` as # `value=""`, so without the `str()` coercion these options would carry no usable @@ -367,3 +390,22 @@ def test_input_select_optgroup_int_values(): ).get_html_string() assert '' in html assert 'value="1" selected' in html + + +def test_input_select_optgroup_label_colliding_with_a_choice_value_raises(): + # An optgroup label and a flat choice value share the top-level mapping, so one of + # the two entries would otherwise disappear. The error says which. + with pytest.raises(ValueError, match="Duplicate choice value or optgroup label"): + # A top level that mixes an optgroup with a flat option is supported at + # runtime, but no arm of `SelectChoicesArg` describes it. + ui.input_select("x", label="L", choices=cast(Any, {0: {1: "one"}, "0": "flat"})) + + +def test_input_select_duplicate_values_across_optgroups_are_allowed(): + # Two groups may hold the same choice value. Duplicate `value` attributes are legal + # HTML and R Shiny permits them; the browser reports only the value, so the options + # are indistinguishable however the server renders them. + html = ui.input_select( + "x", label="L", choices={"A": {0: "zero"}, "B": {"0": "oh"}}, selected=0 + ).get_html_string() + assert html.count('value="0" selected') == 2 From 4e4d7c2241f2c4ce42b4a07e900914ad3c7dbba7 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Wed, 19 Aug 2026 12:56:33 -0600 Subject: [PATCH 12/13] refactor(ui): Build the duplicate-key error from a keyword flag The `keys_are` parameter took the noun that the error message uses, so the switch value carried the message wording. Only two cases exist, so the caller now passes `keys_can_be_optgroup_labels` and one helper builds the message. Both errors are unchanged. --- shiny/ui/_choices.py | 52 +++++++++++++++++++++++---------------- shiny/ui/_input_select.py | 2 +- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/shiny/ui/_choices.py b/shiny/ui/_choices.py index 59b694e16..52598d8ff 100644 --- a/shiny/ui/_choices.py +++ b/shiny/ui/_choices.py @@ -15,7 +15,7 @@ from __future__ import annotations from collections.abc import Collection, Iterable, Mapping -from typing import Any, Literal, TypeVar, Union, cast +from typing import Any, TypeVar, Union, cast # A choice value. The client only ever sees ``str(value)``, so these are the types with # an unambiguous string form -- the ones the choice inputs document and test. @@ -26,30 +26,39 @@ # even `dict[str, str]`. The runtime contract is the same as `ChoiceValue`. ChoiceKey = Any -# What a normalized mapping's keys hold. A select input's `choices` maps optgroup labels -# alongside choice values, so its top level holds both. -ChoiceKeyKind = Literal["choice value", "choice value or optgroup label"] - -_DUPLICATE_KEY_REASONS: dict[ChoiceKeyKind, str] = { - "choice value": ( - "Choice values become HTML `value` attributes, so they must be unique as " - "strings." - ), - "choice value or optgroup label": ( - "A select input's `choices` holds choice values and optgroup labels in one " - "mapping, so both must be unique as strings." - ), -} - V = TypeVar("V") +def duplicate_key_error( + key: str, first: Any, second: Any, *, keys_can_be_optgroup_labels: bool +) -> ValueError: + if keys_can_be_optgroup_labels: + what = "choice value or optgroup label" + reason = ( + "A select input's `choices` (values and optgroup labels) must be unique " + "when converted to strings." + ) + else: + what = "choice value" + reason = ( + "Choice values must be unique when converted to strings strings." + ) + + return ValueError( + f"Duplicate {what} {key!r}: {first!r} and {second!r} are distinct but are " + f"identical as strings. {reason}" + ) + + def normalize_choices_mapping( - x: Mapping[Any, V], *, keys_are: ChoiceKeyKind = "choice value" + x: Mapping[Any, V], *, keys_can_be_optgroup_labels: bool = False ) -> dict[str, V]: """ Coerce choice values (the mapping's keys) to ``str``, preserving order. + A select input's ``choices`` maps optgroup labels alongside choice values, so its + top level sets ``keys_can_be_optgroup_labels``, which only affects the error below. + Raises ------ ValueError @@ -63,10 +72,11 @@ def normalize_choices_mapping( for key, label in x.items(): str_key = str(key) if str_key in normalized: - raise ValueError( - f"Duplicate {keys_are} {str_key!r}: {originals[str_key]!r} and {key!r} " - f"are distinct but are identical once converted to a string. " - + _DUPLICATE_KEY_REASONS[keys_are] + raise duplicate_key_error( + str_key, + originals[str_key], + key, + keys_can_be_optgroup_labels=keys_can_be_optgroup_labels, ) normalized[str_key] = label originals[str_key] = key diff --git a/shiny/ui/_input_select.py b/shiny/ui/_input_select.py index d62314642..9a3ec8538 100644 --- a/shiny/ui/_input_select.py +++ b/shiny/ui/_input_select.py @@ -364,7 +364,7 @@ def _normalize_choices(x: SelectChoicesArg) -> _SelectChoices: # The top-level mapping holds optgroup labels alongside choice values, so a # collision there is reported as either one. - normalized = normalize_choices_mapping(x, keys_are="choice value or optgroup label") + normalized = normalize_choices_mapping(x, keys_can_be_optgroup_labels=True) # The result may mix flat options and optgroups at the top level (e.g. # `{"a": "A", "Group B": {...}}`). That matches neither arm of the From dbb226e208fcfdcd6ced1a3781b5cf8d3893e8bf Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Wed, 19 Aug 2026 13:23:07 -0600 Subject: [PATCH 13/13] refactor(ui): Derive the duplicate-key wording from the colliding labels `normalize_choices_mapping()` needed a flag to say whether a key could be an optgroup label, which put the calling input's shape in the callee's signature. The labels already carry that fact: a key whose label is a `Mapping` heads an optgroup, and only a select input's top-level `choices` holds one. So `duplicate_key_error()` now takes both colliding `(key, label)` pairs and names each side, and the flag is gone. The wording follows the data rather than the caller, which the nested case needs: a select input normalizes each optgroup's contents through the same function, and those keys are all choice values. --- shiny/ui/_choices.py | 49 ++++++++++++++------------ shiny/ui/_input_select.py | 2 +- tests/pytest/test_input_check_radio.py | 2 +- 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/shiny/ui/_choices.py b/shiny/ui/_choices.py index 52598d8ff..3dacfafbd 100644 --- a/shiny/ui/_choices.py +++ b/shiny/ui/_choices.py @@ -30,35 +30,41 @@ def duplicate_key_error( - key: str, first: Any, second: Any, *, keys_can_be_optgroup_labels: bool + key: str, *, first: tuple[Any, Any], second: tuple[Any, Any] ) -> ValueError: - if keys_can_be_optgroup_labels: - what = "choice value or optgroup label" - reason = ( - "A select input's `choices` (values and optgroup labels) must be unique " - "when converted to strings." - ) - else: - what = "choice value" - reason = ( - "Choice values must be unique when converted to strings strings." + """ + Build the error for two keys whose string forms collide. + + ``first`` and ``second`` are the colliding ``(key, label)`` pairs. A key whose label + is a ``Mapping`` heads an optgroup, so it names a group rather than a choice. Only a + select input's top-level ``choices`` can hold one, so the wording follows from the + labels rather than from the calling input. + """ + (first_key, first_label), (second_key, second_label) = first, second + kinds = tuple( + "optgroup label" if isinstance(label, Mapping) else "choice value" + for label in (first_label, second_label) + ) + + if kinds == ("choice value", "choice value"): + return ValueError( + f"Duplicate choice value {key!r}: {first_key!r} and {second_key!r} are " + "distinct but are identical as strings. Choice values must be unique when " + "converted to strings." ) return ValueError( - f"Duplicate {what} {key!r}: {first!r} and {second!r} are distinct but are " - f"identical as strings. {reason}" + f"Duplicate key {key!r} in `choices`: the {kinds[0]} {first_key!r} and the " + f"{kinds[1]} {second_key!r} are distinct but are identical as strings. A select " + "input's `choices` (values and optgroup labels) must be unique when converted " + "to strings." ) -def normalize_choices_mapping( - x: Mapping[Any, V], *, keys_can_be_optgroup_labels: bool = False -) -> dict[str, V]: +def normalize_choices_mapping(x: Mapping[Any, V]) -> dict[str, V]: """ Coerce choice values (the mapping's keys) to ``str``, preserving order. - A select input's ``choices`` maps optgroup labels alongside choice values, so its - top level sets ``keys_can_be_optgroup_labels``, which only affects the error below. - Raises ------ ValueError @@ -74,9 +80,8 @@ def normalize_choices_mapping( if str_key in normalized: raise duplicate_key_error( str_key, - originals[str_key], - key, - keys_can_be_optgroup_labels=keys_can_be_optgroup_labels, + first=(originals[str_key], normalized[str_key]), + second=(key, label), ) normalized[str_key] = label originals[str_key] = key diff --git a/shiny/ui/_input_select.py b/shiny/ui/_input_select.py index 9a3ec8538..a84a118b0 100644 --- a/shiny/ui/_input_select.py +++ b/shiny/ui/_input_select.py @@ -364,7 +364,7 @@ def _normalize_choices(x: SelectChoicesArg) -> _SelectChoices: # The top-level mapping holds optgroup labels alongside choice values, so a # collision there is reported as either one. - normalized = normalize_choices_mapping(x, keys_can_be_optgroup_labels=True) + normalized = normalize_choices_mapping(x) # The result may mix flat options and optgroups at the top level (e.g. # `{"a": "A", "Group B": {...}}`). That matches neither arm of the diff --git a/tests/pytest/test_input_check_radio.py b/tests/pytest/test_input_check_radio.py index a0c536c03..db3564d6f 100644 --- a/tests/pytest/test_input_check_radio.py +++ b/tests/pytest/test_input_check_radio.py @@ -395,7 +395,7 @@ def test_input_select_optgroup_int_values(): def test_input_select_optgroup_label_colliding_with_a_choice_value_raises(): # An optgroup label and a flat choice value share the top-level mapping, so one of # the two entries would otherwise disappear. The error says which. - with pytest.raises(ValueError, match="Duplicate choice value or optgroup label"): + with pytest.raises(ValueError, match="the optgroup label 0 and the choice value"): # A top level that mixes an optgroup with a flat option is supported at # runtime, but no arm of `SelectChoicesArg` describes it. ui.input_select("x", label="L", choices=cast(Any, {0: {1: "one"}, "0": "flat"}))