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

reactive.poll doesn't poll when defined in Express-adjacent modules #1079

Closed
jcheng5 opened this issue Jan 26, 2024 · 8 comments · Fixed by #1172
Closed

reactive.poll doesn't poll when defined in Express-adjacent modules #1079

jcheng5 opened this issue Jan 26, 2024 · 8 comments · Fixed by #1172
Milestone

Comments

@jcheng5
Copy link
Collaborator

jcheng5 commented Jan 26, 2024

While trying to port this app: https://github.com/posit-dev/py-shiny/blob/9ad2ab12ae0054ac745efed78da5fceb1e09fc43/shiny/api-examples/poll/app.py

I tried defining a @reactive.poll in a separate file (data.py), and then using it from the Express app.

Full source code

app.py

from shiny import reactive
from shiny.express import input, render, ui

from data import stock_quotes, SYMBOLS

ui.markdown(
    """
    # `shiny.reactive.poll` demo

    This example app shows how to stream results from a database (in this
    case, an in-memory sqlite3) with the help of `shiny.reactive.poll`.
    """
)

ui.input_selectize("symbols", "Filter by symbol", [""] + SYMBOLS, multiple=True)


def filtered_quotes():
    df = stock_quotes()
    if input.symbols():
        df = df[df["symbol"].isin(input.symbols())]
    return df

@render.express
def table():
    ui.HTML(
        filtered_quotes().to_html(
            index=False, classes="table font-monospace w-auto"
        )
    )

data.py

import asyncio
import random
import sqlite3
from datetime import datetime
from typing import Any, Awaitable

import pandas as pd

from shiny import reactive

SYMBOLS = ["AAA", "BBB", "CCC", "DDD", "EEE", "FFF"]


def timestamp() -> str:
    return datetime.now().strftime("%x %X")


def rand_price() -> float:
    return round(random.random() * 250, 2)


# === Initialize the database =========================================


def init_db(con: sqlite3.Connection) -> None:
    cur = con.cursor()
    try:
        cur.executescript(
            """
            CREATE TABLE stock_quotes (timestamp text, symbol text, price real);
            CREATE INDEX idx_timestamp ON stock_quotes (timestamp);
            """
        )
        cur.executemany(
            "INSERT INTO stock_quotes (timestamp, symbol, price) VALUES (?, ?, ?)",
            [(timestamp(), symbol, rand_price()) for symbol in SYMBOLS],
        )
        con.commit()
    finally:
        cur.close()


conn = sqlite3.connect(":memory:")
init_db(conn)


# === Randomly update the database with an asyncio.task ==============


def update_db(con: sqlite3.Connection) -> None:
    """Update a single stock price entry at random"""

    cur = con.cursor()
    try:
        sym = SYMBOLS[random.randint(0, len(SYMBOLS) - 1)]
        print(f"Updating {sym}")
        cur.execute(
            "UPDATE stock_quotes SET timestamp = ?, price = ? WHERE symbol = ?",
            (timestamp(), rand_price(), sym),
        )
        con.commit()
    finally:
        cur.close()


async def update_db_task(con: sqlite3.Connection) -> Awaitable[None]:
    """Task that alternates between sleeping and updating prices"""
    while True:
        await asyncio.sleep(random.random() * 1.5)
        update_db(con)


asyncio.create_task(update_db_task(conn))


# === Create the reactive.poll object ===============================


def tbl_last_modified() -> Any:
    print("polling")
    df = pd.read_sql_query("SELECT MAX(timestamp) AS timestamp FROM stock_quotes", conn)
    return df["timestamp"].to_list()


@reactive.poll(tbl_last_modified, 0.5)
def stock_quotes() -> pd.DataFrame:
    return pd.read_sql_query("SELECT timestamp, symbol, price FROM stock_quotes", conn)

This loaded, but did not update. The polling function passed to @reactive.poll, tbl_last_modified, did not execute every 0.5 seconds as expected.

The reason for this is because the data.py module is only loaded once (exactly the reason why we created it), but at the time that it's loaded, there's a session active--it happens to be a MockSession. When that MockSession closes, any reactive effects that belong to it shut down--in this case, our @reactive.poll.

This is a tough nut because this automatic closing of effects is necessary for ones that really do belong to the session, but data.py is intended to be global.

I worked around this by passing an explicit session=None argument to reactive.poll().

A fix could be... I don't know, to have a special name for such modules? global.py? And we load them first, outside of any session?

@wch
Copy link
Collaborator

wch commented Jan 27, 2024

It's possible to add an import hook that detects the name of the imported module and does something before and/or after loading that module. For example, here's an import hook in Shinylive for loading seaborn:
https://github.com/posit-dev/shinylive/blob/a941a51e364a8f12ecd6072e48a144e06b2230b2/src/hooks/usePyodide.tsx#L342-L364

So we could add an import hook that sets the current session to None. We do have to be careful about the condition we use when we look for the special module, because we don't want to affect some other module that happens to import a file with the same name.

(Note: we can't use the name global.py because global is a reserved keyword -- import global would result in a syntax error.)

Another possibility: when run_express does the AST traversal and modification, it can look for the specific import name (like import shared) and when it sees it, put it in a with session_context(None): block.

@jcheng5
Copy link
Collaborator Author

jcheng5 commented Jan 29, 2024

Would the import hook be better than preloading it? If it's going to be based on special module names anyway, I think it makes intuitive sense that Shiny would load those modules before loading the first session.

@wch
Copy link
Collaborator

wch commented Jan 30, 2024

I just realized another way to do it is to create a new function for importing shared stuff, similar to importlib.import_module(). Usage would be something like:

from shiny.express import import_shared
shared = import_shared("shared.py")

@jcheng5
Copy link
Collaborator Author

jcheng5 commented Jan 30, 2024

Would that just call importlib.import_module(), but inside of a with session_context(None)? (I think we would want to leave off the .py)

@wch
Copy link
Collaborator

wch commented Jan 30, 2024

Right, it would be something like that.

@jcheng5
Copy link
Collaborator Author

jcheng5 commented Jan 30, 2024

I also think we need a dedicated article talking about global vs. session objects and logic (including but not limited to reactives). Right now I believe the only mention of this is at the end of "Express in depth".

Maybe this would go along with ways of working with data (loading from database, loading from CSV, global vs. session).

cc @cpsievert

@wch wch added this to the v0.8.0 milestone Feb 2, 2024
@wch
Copy link
Collaborator

wch commented Feb 2, 2024

After playing with this for a bit, this is very simple to get to work:

def import_shared(module_name: str) -> ModuleType:
    with session_context(None):
        module = importlib.import_module(module_name)
    return module

# Import a shared.py file
shared = import_shared("shared")

However, there are a few drawbacks:

  • It uses a syntax that differs from a standard import.
  • It doesn't support importing specific objects, as in from shared import x, y
  • It's not easily discoverable.

Another possible approach is to provide a new context manager for people to use whenever they want to run any code that's meant to be shared across sessions. I'll call it app_globals below (though I don't necessarily that's the best name):

# Definition of function (in shiny.express)
@contextmanager
def app_globals():
    with session_context(None):
        yield


# Usage in app
with app_globals():
    from shared import x

All it does is wrap with session_context(None):, but the name app_globals would make some more sense to users. It also gives us room to do other things in the function. For example, it might also make sense to disable the displayhook in the with app_globals() block.

Compared to the import_shared() function above, this:

  • Lets users use standard import syntax.
  • Allows importing specific objects as in from shared import x.
  • Is similarly not easily discoverable.

@wch
Copy link
Collaborator

wch commented Feb 29, 2024

Possible path forward:

  • Add a magical import name
  • If it exists, load that file with session_context(None) at the top of wrap_express_app
  • If it throws an error, make sure to display it

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 a pull request may close this issue.

2 participants