fix(py): type-check tests and examples; enforce the formatter - #306
Merged
Merged
Conversation
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`.
dict every app returns; check tests and examples
schloerke
force-pushed
the
schloerke/py-lint-coverage-gaps
branch
from
September 12, 2026 22:36
ea2b22c to
21d5f27
Compare
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 byApp(ui=)It declared
HTMLTextDocument, butApp(ui=)accepts only py-shiny'sPageHtmlDocumentsubclass.shiny.App(page_react_html(...), server)— a documented path — failed to type-check while working fine at runtime. The class is imported fromshiny.ui._pagebecause py-shiny exportspage_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:
Jsonifiablerejects thedictevery app returns → posit-dev/py-shiny#2497Jsonifiablespells its container arms asListandDict. Both are invariant in their element types, so a render function returningdict[str, int]is not assignable to it, even though every element is itselfJsonifiable:The single most ordinary thing a
reactive_outputdoes was a type error in five example apps.I first fixed this locally with a covariant
Mapping/Sequencealias, 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 aRenderer[Jsonifiable].So each site carries a suppression naming the issue instead:
grep 2497finds all seven when it lands. Runtime behavior is unaffected — it is an annotation bug only.The four tooling gaps
py-check-formatranruff checkonly, neverruff format --check— the formatter's output was unenforced, andtest_output_error.pyhad already driftedruff format --check; reformatted the drifted filepkg-py, leavingexamples/unlinted and unformatted — despite being shipped, read as documentation, and run by the default pytest invocation$(PATHS_PY) = pkg-py examples[tool.pyright] includewas onlypkg-py/src/shinyreact, so no test or example was type-checkedpkg-py/testsandexamples, and fixed the 50 pre-existing errorstests-e2easked for ashiny[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 syncpytest-playwrightalready pulls inplaywrightThe pre-existing errors, fixed in place
test_input_handler.py/test_dep_discovery.py—NO_NAME/NO_SESSION/ANY_NAMEcasts, because these tests deliberately passNonefor arguments the handlers ignore, which is part of what they assert.test_dep.py—dep_source()/first_script()/first_stylesheet()helpers.HTMLDependencynormalizes 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 takesMapping, same invariance root cause as #2497.test_set_react_page.py—RenderedHTML, the TypedDictrender()actually returns.04-shadcn/app.py—.loc[:, [...]]instead of[[...]]; pandas types the latter asSeries | Unknown, andSeries.to_dict()takes noorient=.Verification
make py-check— ruff lint + format clean, pyright 0 errors over package + tests + examples, 198 passedmake py-test-e2e(chromium) — 28 passeduv sync --all-extras --all-groups— no extras warningFEATURES.mdupdated for both thepage_react_htmlreturn type and the #2497 suppression