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 Loading Spinners to Components in home.py #143

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 40 additions & 11 deletions app_sidebar_collapsible.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ def make_controls():
id='global-loading',
type='default',
fullscreen=True,
overlay_style={"visibility": "visible", "filter": "blur(2px)"},
overlay_style={"visibility": "visible", "opacity": 0.5},
style={"background-color": "transparent"},
children=html.Div(dash.page_container, style={
"margin-left": "5rem",
Expand All @@ -241,17 +241,46 @@ def make_home_page(): return [
]


def make_layout(): return html.Div([
dcc.Location(id='url', refresh=False),
dcc.Store(id='store-trips', data={}),
dcc.Store(id='store-uuids', data={}),
dcc.Store(id='store-excluded-uuids', data={}), # list of UUIDs from excluded subgroups
dcc.Store(id='store-demographics', data={}),
dcc.Store(id='store-trajectories', data={}),
html.Div(id='page-content', children=make_home_page()),
])
def make_layout():
layout = html.Div([
html.Div(id='home-page-load', style={'display': 'none'}),
dcc.Location(id='url', refresh=False),
dcc.Store(id='store-trips', data={}),
dcc.Store(id='store-uuids', data={}),
dcc.Store(id='store-excluded-uuids', data={}),
dcc.Store(id='store-demographics', data={}),
dcc.Store(id='store-trajectories', data={}),
html.Div(id='page-content', children=make_home_page()),

])
logging.debug("Main layout component IDs: %s", get_all_component_ids(layout))
return layout

Comment on lines +244 to +258
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the only purpose of this was for your own debugging, please revert

app.layout = make_layout

def get_all_component_ids(component):
ids = []
if hasattr(component, 'id') and component.id:
ids.append(component.id)
if hasattr(component, 'children'):
children = component.children
if isinstance(children, list):
for child in children:
ids.extend(get_all_component_ids(child))
else:
ids.extend(get_all_component_ids(children))
return ids



app.validation_layout = html.Div([
make_layout(),
*[page['layout'] for page in dash.page_registry.values()]
])

logging.debug("Validation layout component IDs: %s", get_all_component_ids(app.validation_layout))


# make the 'filters' menu collapsible
@app.callback(
Output("collapse-filters", "is_open"),
Expand Down Expand Up @@ -385,4 +414,4 @@ def display_page(search):
SESSION_COOKIE_HTTPONLY=True
)
logging.debug("after override, current server config = %s" % server.config)
app.run_server(debug=envDebug, host='0.0.0.0', port=envPort)
app.run_server(debug=envDebug, host='0.0.0.0', port=envPort)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace change

2 changes: 1 addition & 1 deletion pages/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
html.Div(id='tabs-content'),
dcc.Store(id='selected-tab', data='tab-uuids-datatable'), # Store to hold selected tab
dcc.Interval(id='interval-load-more', interval=20000, n_intervals=0), # default loading at 10s, can be lowered or hightened based on perf (usual process local is 3s)
dcc.Store(id='store-uuids', data=[]), # Store to hold the original UUIDs data
#dcc.Store(id='store-uuids', data=[]), # Store to hold the original UUIDs data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#dcc.Store(id='store-uuids', data=[]), # Store to hold the original UUIDs data

Yes, you can remove this Store from this page. As you have correctly determined, the store-uuids that comes from app_sidebar_collapsible.py is global and redeclaring it here will cause issues

dcc.Store(id='store-loaded-uuids', data={'data': [], 'loaded': False}), # Store to track loaded data
# RadioItems for key list switch, wrapped in a div that can hide/show
html.Div(
Expand Down
216 changes: 131 additions & 85 deletions pages/home.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
"""
Note that the callback will trigger even if prevent_initial_call=True. This is because dcc.Location must
be in app.py. Since the dcc.Location component is not in the layout when navigating to this page, it triggers the callback.
The workaround is to check if the input value is None.

"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment did not make sense or seem relevant to me either.
We looked back at the blame to see what commit it was added (6cf3d74#diff-e1d0f200a354a62599c454209b8b5815e380dd4ce4394c3e57a8f1a2e718113d) and did not find anything to justify keeping the comment.

So I am in support of removing the comment

from uuid import UUID
from dash import dcc, html, Input, Output, callback, register_page
from dash import dcc, html, Input, Output, callback, register_page, no_update
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me what no_update does or why it's added. Can you add a comment in the code with an explanation (or a link to Dash documentation that explains it)?

import dash_bootstrap_components as dbc
import dash_mantine_components as dmc

import plotly.express as px

Expand All @@ -31,25 +26,49 @@
"margin": "auto",
}

layout = html.Div(
[
dcc.Markdown(intro),

# Cards
dbc.Row([
dbc.Col(id='card-users'),
dbc.Col(id='card-active-users'),
dbc.Col(id='card-trips')
]),

# Plots
dbc.Row([
dcc.Graph(id="fig-sign-up-trend"),
dcc.Graph(id="fig-trips-trend"),
])
]
)
def wrap_with_skeleton(component_id, height, children_component):
return dmc.Skeleton(
height=height,
visible=True, # Initially visible
id=f'skeleton-{component_id}',
children=children_component
)

layout = dmc.MantineProvider(
theme={
"colorScheme": "light", # or "dark"
},

children=html.Div(
[
dcc.Markdown(intro),

# Cards Section
dbc.Row([
dbc.Col(
wrap_with_skeleton('users', 100, html.Div(id='card-users'))
),
dbc.Col(
wrap_with_skeleton('active-users', 100, html.Div(id='card-active-users'))
),
dbc.Col(
wrap_with_skeleton('trips', 100, html.Div(id='card-trips'))
),
], className="mb-4"), # Add margin-bottom for spacing

# Plots Section
dbc.Row([
dbc.Col(
wrap_with_skeleton('sign-up-trend', 300, dcc.Graph(id="fig-sign-up-trend"))
),
dbc.Col(
wrap_with_skeleton('trips-trend', 300, dcc.Graph(id="fig-trips-trend"))
),
]),
],
style={"padding": "20px"} # Optional padding for aesthetics
)
)

def compute_sign_up_trend(uuid_df):
uuid_df['update_ts'] = pd.to_datetime(uuid_df['update_ts'], utc=True)
Expand All @@ -62,7 +81,6 @@ def compute_sign_up_trend(uuid_df):
)
return res_df


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to minimize whitespace changes

Especially because IIRC, having 2 line breaks between functions is the PEP8 spec

def compute_trips_trend(trips_df, date_col):
trips_df[date_col] = pd.to_datetime(trips_df[date_col], utc=True)
trips_df[date_col] = pd.DatetimeIndex(trips_df[date_col]).date
Expand All @@ -75,7 +93,6 @@ def compute_trips_trend(trips_df, date_col):
)
return res_df


def find_last_get(uuid_list):
uuid_list = [UUID(npu) for npu in uuid_list]
last_item = list(edb.get_timeseries_db().aggregate([
Expand All @@ -86,7 +103,6 @@ def find_last_get(uuid_list):
]))
return last_item


def get_number_of_active_users(uuid_list, threshold):
last_get_entries = find_last_get(uuid_list)
number_of_active_users = 0
Expand All @@ -98,7 +114,6 @@ def get_number_of_active_users(uuid_list, threshold):
number_of_active_users += 1
return number_of_active_users


def generate_card(title_text, body_text, icon):
card = dbc.CardGroup([
dbc.Card(
Expand All @@ -117,73 +132,104 @@ def generate_card(title_text, body_text, icon):
])
return card

def generate_barplot(data, x, y, title):
fig = px.bar()
if data is not None and not data.empty:
fig = px.bar(data, x=x, y=y)
fig.update_layout(title=title)
return fig

@callback(
Output('card-users', 'children'),
Input('store-uuids', 'data'),
)
def update_card_users(store_uuids):
number_of_users = store_uuids.get('length') if has_permission('overview_users') else 0
card = generate_card("# Users", f"{number_of_users} users", "fa fa-users")
return card


@callback(
Output('card-active-users', 'children'),
Input('store-uuids', 'data'),
[
Output('card-users', 'children'),
Output('skeleton-users', 'visible'),
Output('card-active-users', 'children'),
Output('skeleton-active-users', 'visible'),
Output('card-trips', 'children'),
Output('skeleton-trips', 'visible'),
Output('fig-sign-up-trend', 'figure'),
Output('skeleton-sign-up-trend', 'visible'),
Output('fig-trips-trend', 'figure'),
Output('skeleton-trips-trend', 'visible'),
],
[
Input('store-uuids', 'data'),
Input('store-trips', 'data'),
Input('date-picker', 'start_date'),
Input('date-picker', 'end_date'),
Input('url', 'pathname'),
Input('home-page-load', 'children')
]
)
def update_card_active_users(store_uuids):
uuid_df = pd.DataFrame(store_uuids.get('data'))
def update_all_components(store_uuids, store_trips, start_date, end_date, pathname, _):
Comment on lines 142 to +164
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am seeing that we used to have multiple, smaller callbacks for each card. Now we have one big callback that handles everything– which is worse from a code tidiness standpoint.
Can you explain the rationale behind this?

if pathname != "/":
return [no_update] * 10

# Initialize all outputs
card_users = no_update
skeleton_users = no_update
card_active_users = no_update
skeleton_active_users = no_update
card_trips = no_update
skeleton_trips = no_update
fig_sign_up_trend = no_update
skeleton_sign_up_trend = no_update
fig_trips_trend = no_update
skeleton_trips_trend = no_update

# Flags to check if each component's data is ready
all_ready = True

# Update Users Card
if store_uuids is None or not has_permission('overview_users'):
number_of_users = 0
else:
number_of_users = store_uuids.get('length', 0)
card_users = generate_card("# Users", f"{number_of_users} users", "fa fa-users")

# Update Active Users Card
uuid_df = pd.DataFrame(store_uuids.get('data')) if store_uuids else pd.DataFrame()
number_of_active_users = 0
if not uuid_df.empty and has_permission('overview_active_users'):
one_day = 24 * 60 * 60
number_of_active_users = get_number_of_active_users(uuid_df['user_id'], one_day)
card = generate_card("# Active users", f"{number_of_active_users} users", "fa fa-person-walking")
return card


@callback(
Output('card-trips', 'children'),
Input('store-trips', 'data'),
)
def update_card_trips(store_trips):
number_of_trips = store_trips.get('length') if has_permission('overview_trips') else 0
card = generate_card("# Confirmed trips", f"{number_of_trips} trips", "fa fa-angles-right")
return card
card_active_users = generate_card("# Active users", f"{number_of_active_users} users", "fa fa-person-walking")

# Update Trips Card
number_of_trips = store_trips.get('length') if (store_trips and has_permission('overview_trips')) else 0
card_trips = generate_card("# Confirmed trips", f"{number_of_trips} trips", "fa fa-angles-right")

def generate_barplot(data, x, y, title):
fig = px.bar()
if data is not None:
fig = px.bar(data, x=x, y=y)
fig.update_layout(title=title)
return fig


@callback(
Output('fig-sign-up-trend', 'figure'),
Input('store-uuids', 'data'),
)
def generate_plot_sign_up_trend(store_uuids):
df = pd.DataFrame(store_uuids.get("data"))
# Update Sign-Up Trend Figure
df = pd.DataFrame(store_uuids.get("data")) if store_uuids else pd.DataFrame()
trend_df = None
if not df.empty and has_permission('overview_signup_trends'):
trend_df = compute_sign_up_trend(df)
fig = generate_barplot(trend_df, x = 'date', y = 'count', title = "Sign-ups trend")
return fig
fig_sign_up_trend = generate_barplot(trend_df, x='date', y='count', title="Sign-ups trend")


@callback(
Output('fig-trips-trend', 'figure'),
Input('store-trips', 'data'),
Input('date-picker', 'start_date'), # these are ISO strings
Input('date-picker', 'end_date'), # these are ISO strings
)
def generate_plot_trips_trend(store_trips, start_date, end_date):
df = pd.DataFrame(store_trips.get("data"))
trend_df = None
# Update Trips Trend Figure
trips_df = pd.DataFrame(store_trips.get("data")) if store_trips else pd.DataFrame()
trend_trips_df = None
(start_date, end_date) = iso_to_date_only(start_date, end_date)
if not df.empty and has_permission('overview_trips_trend'):
trend_df = compute_trips_trend(df, date_col = "trip_start_time_str")
fig = generate_barplot(trend_df, x = 'date', y = 'count', title = f"Trips trend({start_date} to {end_date})")
return fig
if not trips_df.empty and has_permission('overview_trips_trend'):
trend_trips_df = compute_trips_trend(trips_df, date_col="trip_start_time_str")
fig_trips_trend = generate_barplot(trend_trips_df, x='date', y='count', title=f"Trips trend ({start_date} to {end_date})")

# Set all skeletons to False since all components are updated
skeleton_users = False
skeleton_active_users = False
skeleton_trips = False
skeleton_sign_up_trend = False
skeleton_trips_trend = False

return [
card_users,
skeleton_users,
card_active_users,
skeleton_active_users,
card_trips,
skeleton_trips,
fig_sign_up_trend,
skeleton_sign_up_trend,
fig_trips_trend,
skeleton_trips_trend,
]