Skip to content

fix(py): type-check tests and examples; enforce the formatter - #306

Merged
schloerke merged 3 commits into
mainfrom
schloerke/py-lint-coverage-gaps
Sep 12, 2026
Merged

schloerke merged 3 commits into
mainfrom
schloerke/py-lint-coverage-gaps

Conversation

@schloerke

@schloerke schloerke commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #305, which turned up four gaps in the Python tooling. Closing them surfaced two annotations that were unusable from user code — which is the argument for closing them. One is ours; one is upstream.

Ours: page_react_html() was not accepted by App(ui=)

It declared HTMLTextDocument, but App(ui=) accepts only py-shiny's PageHtmlDocument subclass. shiny.App(page_react_html(...), server) — a documented path — failed to type-check while working fine at runtime. The class is imported from shiny.ui._page because py-shiny exports page_html() but not its return class.

Pinned by pkg-py/tests/test_typing.py, which asserts nothing at runtime — pyright failing is the test, the only thing that can catch an over-narrow annotation.

Upstream: Jsonifiable rejects the dict every app returns → posit-dev/py-shiny#2497

Jsonifiable spells its container arms as List and Dict. Both are invariant in their element types, so a render function returning dict[str, int] is not assignable to it, even though every element is itself Jsonifiable:

error: Argument of type "() -> dict[str, int]" cannot be assigned to parameter "_fn"
  of type "(() -> Jsonifiable) | (() -> Awaitable[Jsonifiable]) | None"

The single most ordinary thing a reactive_output does was a type error in five example apps.

I first fixed this locally with a covariant Mapping/Sequence alias, then reverted that in favor of filing posit-dev/py-shiny#2497, with a reproducer that uses only py-shiny's documented custom-renderer extension point — no shinyreact involved. A local alias would have fixed our five call sites and left the same trap for everyone else writing a Renderer[Jsonifiable].

So each site carries a suppression naming the issue instead:

# py-shiny#2497: `Jsonifiable`'s `dict`/`list` arms are invariant, so a
# `dict` return is not assignable to it. Drop the ignore when that lands.
@reactive_output  # pyright: ignore[reportArgumentType]
def dist_data():
    return histogram(waiting, input.bins())

grep 2497 finds all seven when it lands. Runtime behavior is unaffected — it is an annotation bug only.

The four tooling gaps

Gap Fix
1 py-check-format ran ruff check only, never ruff format --check — the formatter's output was unenforced, and test_output_error.py had already drifted added ruff format --check; reformatted the drifted file
2 Both ruff targets scoped to pkg-py, leaving examples/ unlinted and unformatted — despite being shipped, read as documentation, and run by the default pytest invocation both now cover $(PATHS_PY) = pkg-py examples
3 [tool.pyright] include was only pkg-py/src/shinyreact, so no test or example was type-checked added pkg-py/tests and examples, and fixed the 50 pre-existing errors
4 tests-e2e asked for a shiny[playwright] extra py-shiny does not have (theme, otel, test, dev, doc, add-test) — uv reports this as a warning, not an error, on every sync dropped the extra; pytest-playwright already pulls in playwright

The pre-existing errors, fixed in place

  • test_input_handler.py / test_dep_discovery.py — NO_NAME / NO_SESSION / ANY_NAME casts, because these tests deliberately pass None for arguments the handlers ignore, which is part of what they assert.
  • test_dep.py — dep_source() / first_script() / first_stylesheet() helpers. HTMLDependency normalizes these attributes in __init__ but keeps them declared as the wider argument types; narrowing once lets the assertions read as assertions instead of type gymnastics.
  • test_bookmark_restore.py — local _config_html() helper takes Mapping, same invariance root cause as #2497.
  • test_set_react_page.py — RenderedHTML, the TypedDict render() actually returns.
  • 04-shadcn/app.py — .loc[:, [...]] instead of [[...]]; pandas types the latter as Series | Unknown, and Series.to_dict() takes no orient=.

Verification

  • make py-check — ruff lint + format clean, pyright 0 errors over package + tests + examples, 198 passed
  • make py-test-e2e (chromium) — 28 passed
  • uv sync --all-extras --all-groups — no extras warning
  • FEATURES.md updated for both the page_react_html return type and the #2497 suppression

Widening pyright past `pkg-py/src/shinyreact` surfaced two annotations
that were unusable from user code -- which is the argument for widening
it. `pkg-py/tests` and `examples` are the only place the *caller's* view
of the API is exercised, so nothing else could have caught either.

- `reactive_output` and `send_message(data=)` were typed `Jsonifiable`,
  whose containers are `dict` and `list`. Both are invariant in their
  element types, so a render function returning the natural
  `dict[str, int]` was not assignable to it: the single most ordinary
  thing a `reactive_output` does was a type error at every call site,
  five example apps included. They now take `JsonValue`, the same union
  spelled with covariant `Mapping` / `Sequence`; `Jsonifiable` stays the
  type on the way out to Shiny.
- `page_react_html()` declared `HTMLTextDocument`, but `App(ui=)`
  accepts only its `PageHtmlDocument` subclass -- so the documented
  plain-`shiny.App` path failed to type-check.

`pkg-py/tests/test_typing.py` pins both. It asserts nothing at runtime;
pyright failing is the test. Reverting either fix produces 13 errors.

Three tooling gaps closed alongside:

- `py-check-format` ran `ruff check` only, never `ruff format --check`,
  so the formatter's output was unenforced -- and one file had already
  drifted.
- Both ruff targets scoped to `pkg-py`, leaving `examples/` unlinted and
  unformatted despite being shipped, read as documentation, and run by
  the default pytest invocation. Both now cover `$(PATHS_PY)`.
- The `tests-e2e` group asked for a `shiny[playwright]` extra py-shiny
  does not have, which uv reports as a warning rather than an error on
  every sync -- exactly how a real extras problem would hide.

The remaining pre-existing errors in tests and examples are fixed in
place: casts where a test deliberately passes `None` for an argument the
handler ignores, narrowing helpers in `test_dep.py` for
`HTMLDependency`'s wider-than-actual attribute types, and `.loc[:, ...]`
in `04-shadcn` because pandas types `df[[...]]` as `Series | Unknown`.
Widening pyright past `pkg-py/src/shinyreact` surfaced one annotation of
ours that was unusable from user code, and one of py-shiny's.

`page_react_html()` declared `HTMLTextDocument`, but `App(ui=)` accepts
only its `PageHtmlDocument` subclass, so the documented plain-`shiny.App`
path failed to type-check. Fixed here, and pinned by
`pkg-py/tests/test_typing.py`, which asserts nothing at runtime --
pyright failing is the test.

The other is upstream: `Jsonifiable`'s `dict` / `list` arms are
invariant in their element types, so a render function returning
`dict[str, int]` is not assignable to it -- the single most ordinary
thing a `reactive_output` does, a type error in five example apps. That
is posit-dev/py-shiny#2497; suppressed per call site with a
`# pyright: ignore[reportArgumentType]` naming the issue rather than
worked around locally, so the fix lands once, upstream. Grep `2497` to
find every site when it does.

Three tooling gaps closed alongside:

- `py-check-format` ran `ruff check` only, never `ruff format --check`,
  so the formatter's output was unenforced -- and one file had already
  drifted.
- Both ruff targets scoped to `pkg-py`, leaving `examples/` unlinted and
  unformatted despite being shipped, read as documentation, and run by
  the default pytest invocation. Both now cover `$(PATHS_PY)`.
- The `tests-e2e` group asked for a `shiny[playwright]` extra py-shiny
  does not have, which uv reports as a warning rather than an error on
  every sync -- exactly how a real extras problem would hide.

The remaining pre-existing errors in tests and examples are fixed in
place: casts where a test deliberately passes `None` for an argument the
handler ignores, narrowing helpers in `test_dep.py` for
`HTMLDependency`'s wider-than-actual attribute types, and `.loc[:, ...]`
in `04-shadcn` because pandas types `df[[...]]` as `Series | Unknown`.
@schloerke schloerke changed the title fix(py): accept the dict every app returns; check tests and examples fix(py): type-check tests and examples; enforce the formatter Sep 12, 2026
@schloerke
schloerke force-pushed the schloerke/py-lint-coverage-gaps branch from ea2b22c to 21d5f27 Compare September 12, 2026 22:36
The private shiny.ui._page import is used only in a return annotation, so
guard it under TYPE_CHECKING: an upstream rename becomes a pyright error
instead of an ImportError at app startup. Also correct the py-shiny#2497
note on test_passthrough_list, which returns a list, not a dict.
@schloerke
schloerke enabled auto-merge (squash) September 12, 2026 22:48
@schloerke
schloerke merged commit e23d826 into main Sep 12, 2026
5 checks passed
@schloerke
schloerke deleted the schloerke/py-lint-coverage-gaps branch September 12, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant