Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fixes for type stub generation #828

Merged
merged 1 commit into from
Dec 19, 2023
Merged

Add fixes for type stub generation #828

merged 1 commit into from
Dec 19, 2023

Conversation

wch
Copy link
Collaborator

@wch wch commented Nov 28, 2023

When shiny type stubs are generated by pyright, and they are used for types (instead of using the shiny source code directly), we have some typing errors, which are visible in shinylive:

image

This is because some of the .pyi files generated by pyright lack necessary type information. This PR fixes most cases of missing types in the .pyi files, but not all.

@schloerke in the future we should have an automated test which runs pyright on the generated .pyi files, as well as the examples (but using the shiny type stubs instead of shiny sources directly).


To illustrate what's going on, here's the relevant part of OutputTransformer from _transformer.py before the change:

class OutputTransformer(Generic[IT, OT, P]):
    def __init__(
        self,
        fn: OutputTransformerFn[IT, P, OT],
    ) -> None:
        self._fn = fn
        self.ValueFn = ValueFn[IT]
        self.OutputRenderer = OutputRenderer[OT]
        self.OutputRendererDecorator = OutputRendererDecorator[IT, OT]

This is the content of the resulting _transformer.pyi file:

class OutputTransformer(Generic[IT, OT, P]):
    def __init__(self, fn: OutputTransformerFn[IT, P, OT]) -> None:
        ...

Notice that it loses all information about self._fn, self.ValueFn, self.OutputRenderer, and self.OutputRendererDecorator.

This is what the OutputTransformer looks like after the change:

class OutputTransformer(Generic[IT, OT, P]):
    fn: OutputTransformerFn[IT, P, OT]
    ValueFn: Type[ValueFn[IT]]
    OutputRenderer: Type[OutputRenderer[OT]]
    OutputRendererDecorator: Type[OutputRendererDecorator[IT, OT]]

    def __init__(
        self,
        fn: OutputTransformerFn[IT, P, OT],
    ) -> None:
        self._fn = fn
        self.ValueFn = ValueFn[IT]
        self.OutputRenderer = OutputRenderer[OT]
        self.OutputRendererDecorator = OutputRendererDecorator[IT, OT]

The class-level variables like fn and OutputRenderer aren't actually used by any of the code, but pyright uses them for generating type stubs. This is the resulting _transformer.pyi file:

class OutputTransformer(Generic[IT, OT, P]):
    fn: OutputTransformerFn[IT, P, OT]
    ValueFn: Type[ValueFn[IT]]
    OutputRenderer: Type[OutputRenderer[OT]]
    OutputRendererDecorator: Type[OutputRendererDecorator[IT, OT]]

    def __init__(self, fn: OutputTransformerFn[IT, P, OT]) -> None:
        ...

That change makes the red underlines go away in shinylive.

@wch wch requested a review from schloerke November 28, 2023 20:54
@wch
Copy link
Collaborator Author

wch commented Nov 28, 2023

Update: there still seems to be red underlining when I build shinylive with this change:

image

I'm not sure if it's because shinylive is stuck on an old version of pyright, or if it's because it's not actually fixed here. Note that if I load up the same app in VS Code and use the shiny type stubs (instead of shiny source code), then it does not underline the @output:

image

App source:

from shiny import App, render, ui, Outputs, Session, Inputs

app_ui = ui.page_fluid(
    ui.output_text("txt"),
)

def server(input: Inputs, output: Outputs, session: Session):
    @output
    @render.text()
    def txt():
        return "boo"

app = App(app_ui, server)

@wch wch merged commit 028721d into main Dec 19, 2023
26 checks passed
@wch wch deleted the fix-typestub-types branch December 19, 2023 21:37
schloerke added a commit that referenced this pull request Jan 8, 2024
* main: (24 commits)
  Use dynamic version of py-shiny for deploy tests (#970)
  Add underscores to hide some imports (#978)
  Add rsconnect json files(shinyapps.io tests) and folium tests (#928)
  Express' `value_box()` no longer includes named positional args (#966)
  Include `tooltip()` and `popover()` in express (#949)
  Remove extra call to run_express()
  Call `tagify()` early to intercept `AttributeErrors` (#941)
  Don't pass sidebar twice to navbar_page
  Update changelog
  Update changelog
  Switch from `requests` to `urllib` (#940)
  Bump version to 0.6.1.1
  Fix docstring for page_opts
  Fix API doc sections for Express
  Smarter, lazier, and more complete page default/api for express (#893)
  Change `express.layout` to `express.ui` (#904)
  Remove `@output` from examples (#790)
  feat: Allow for `App` `server=` to take `input` only (#920)
  Add fixes for type stub generation (#828)
  Move quarto express docs to bottom
  ...
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.

None yet

1 participant