plotly / dash

Data Apps & Dashboards for Python. No JavaScript Required.
https://plotly.com/dash
MIT License
21.37k stars 2.06k forks source link

Expose `dash.NoUpdate` for Type Hinting #2799

Open 2Ryan09 opened 7 months ago

2Ryan09 commented 7 months ago

Enable more comprehensive type hinting of dash callback methods by exposing dash._callback.NoUpdate.

Given callback() and clientside_callback() are already imported up, is there any reason not to also expose NoUpdate?

A Minimal Dash App with comprehensive python typing:

# import NoUpdate alongside any other top-level import
from dash import Dash, NoUpdate, html, dcc, callback, Output, Input
import plotly.express as px
import pandas as pd

# TYPE_CHECKING block
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from plotly.graph_objects import Figure

df = pd.read_csv('https://raw.githubusercontent.com/plotly/datasets/master/gapminder_unfiltered.csv')

app = Dash(__name__)

app.layout = html.Div([
    html.H1(children='Title of Dash App', style={'textAlign':'center'}),
    dcc.Dropdown(df.country.unique(), 'Canada', id='dropdown-selection'),
    dcc.Graph(id='graph-content')
])

@callback(
    Output('graph-content', 'figure'),
    Input('dropdown-selection', 'value')
)
def update_graph(value: str | None) -> Figure | NoUpdate:  # Complete Input and Output typing, no room for error
    dff = df[df.country==value]
    return px.line(dff, x='year', y='pop')

if __name__ == '__main__':
    app.run(debug=True)
alexcjohnson commented 7 months ago

Thanks @2Ryan09 - makes sense, but I’m worried about confusion between the no_update value and the NoUpdate class (of which normally no_update is the lone instance). Is there a relevant convention we can use to make this distinction clear? Rename the class NoUpdateType or something, ala None / NoneType?

2Ryan09 commented 7 months ago

Unfortunately I don't believe there are any accepted conventions for this other than the they styled as CapWords.

NoUpdateType seems like a perfectly reasonable solution to me. Will update the PR.

2Ryan09 commented 7 months ago

On a higher note, should NoUpdate be a TypedDict?

alexcjohnson commented 7 months ago

On a higher note, should NoUpdate be a TypedDict?

Because of its JSON serialization? @T4rk1n correct me if I'm wrong but I think that's just for internal use with background callbacks, shouldn't be visible to users.

2Ryan09 commented 7 months ago

On a higher note, should NoUpdate be a TypedDict?

Because of its JSON serialization? @T4rk1n correct me if I'm wrong but I think that's just for internal use with background callbacks, shouldn't be visible to users.

Because, from what I can gather, NoUpdate can be entirely represented by {"_dash_no_update": "_dash_no_update"} and two methods within either compare it to that dictionary or just check it is of that type.

T4rk1n commented 7 months ago

Is there a relevant convention we can use to make this distinction clear? Rename the class NoUpdateType or something, ala None / NoneType?

NoneType is a special case, it isn't actually defined it's just the return type of type(None). The correct typing for None is -> None not NoneType.

Because, from what I can gather, NoUpdate can be entirely represented by {"_dash_no_update": "_dash_no_update"} and two methods within either compare it to that dictionary or just check it is of that type.

Yes, I guess we could add the check in to __eq__ of NoUpdate to get == operator to work with the dict syntax.

NoUpdate on the user side is mostly just a singleton and the api we have is return dash.no_update or the old raise PreventUpdate. I can see some error coming from return NoUpdate instead of return NoUpdate() because isinstance(NoUpdate, (NoUpdate,)) is false so maybe add an additional check for that case in the staticmethod.