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

Shiny Express errors when trying to render DuckDBPyConnection object #1771

Open
wch opened this issue Nov 13, 2024 · 3 comments
Open

Shiny Express errors when trying to render DuckDBPyConnection object #1771

wch opened this issue Nov 13, 2024 · 3 comments

Comments

@wch
Copy link
Collaborator

wch commented Nov 13, 2024

The following app crashes on start because of the con.register() at the top level. Live app. (It could be suppressed by using with ui.hold():.)

from shiny.express import input, render, ui
import pandas as pd
import duckdb

# Create a sample DataFrame
df = pd.DataFrame({
    'id': range(1, 6),
    'name': ['Alice', 'Bob', 'Charlie', 'David', 'Eve'],
    'age': [25, 30, 35, 28, 22]
})


# Register DataFrame with DuckDB
con = duckdb.connect(':memory:')
con.register('people', df)

The error message is:

Traceback (most recent call last):
  File "<exec>", line 393, in _start_app
  File "<exec>", line 366, in __init__
  File "/lib/python3.12/site-packages/shiny/express/_run.py", line 62, in wrap_express_app
    app_module = importlib.import_module(package_name)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python312.zip/importlib/__init__.py", line 90, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
  File "/lib/python3.12/site-packages/shiny/express/_run.py", line 102, in exec_module
    module.app = create_express_app(  # pyright: ignore[reportAttributeAccessIssue]
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.12/site-packages/shiny/express/_run.py", line 132, in create_express_app
    app_ui = run_express(file, package_name).tagify()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.12/site-packages/shiny/express/_run.py", line 231, in run_express
    get_top_level_recall_context_manager().__exit__(None, None, None)
  File "/lib/python3.12/site-packages/shiny/express/_recall_context.py", line 48, in __exit__
    res = self.fn(*self.args, **self.kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.12/site-packages/shiny/ui/_page.py", line 719, in page_auto
    return page_fn(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.12/site-packages/shiny/ui/_page.py", line 775, in _page_auto_fixed
    return page_fixed(
           ^^^^^^^^^^^
  File "/lib/python3.12/site-packages/shiny/ui/_page.py", line 518, in page_fixed
    div({"class": "container"}, *args, **kwargs),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.12/site-packages/htmltools/tags.py", line 731, in div
    return Tag("div", *args, _add_ws=_add_ws, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.12/site-packages/htmltools/_core.py", line 679, in __init__
    self.children = TagList(*kids)
                    ^^^^^^^^^^^^^^
  File "/lib/python3.12/site-packages/htmltools/_core.py", line 281, in __init__
    super().__init__(_tagchilds_to_tagnodes(args))
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.12/site-packages/htmltools/_core.py", line 1936, in _tagchilds_to_tagnodes
    raise TypeError(
TypeError: Invalid tag item type: <class 'duckdb.duckdb.DuckDBPyConnection'>. Consider calling str() on this value before treating it as a tag item.

Shiny tries to render the object, but fails. A few possible ways of dealing with this:

  • We should have a more useful error message and suggest how to fix.
  • We could automatically not render objects that Shiny doesn't know how to render.
@gadenbuie
Copy link
Collaborator

I have a feeling that we could go all the way to the other side of the spectrum from where we are currently and only include things in the UI that are tagifiable or literal strings. Are there any other classes of objects that we automatically render in any meaningful way?

@wch
Copy link
Collaborator Author

wch commented Nov 14, 2024

Looking back at the code for RecallContextManager, I believe this is this the reason why we made things the way they are currently.

Each time there's a with shiny.express.ui.div() or with shiny.express.ui.sidebar(), this enters a recall context, where each line is evaluated, and then if it returns a value, that value is appended to a list of stored values (note that there are some exceptions which don't get appended to that list, like function definitions). When it exits the with block, the actual function (which was wrapped by the recall context manager) is invoked with those values as arguments. For example, if this is the code:

from shiny.express import ui

with ui.div():
    "Hello"
    ui.input_text("in", "Enter text")

This translates to:

from shiny import ui

ui.div("Hello", ui.input_text("in", "Enter text"))

All of the content in a Shiny Express app is in an implicitly-defined top-level recall context manager. This is by default page_auto(). There's no with statement for it, but you can imagine the entire file content put inside of a with ui.page_auto().

In the example above, values from each line in the with block are a string, and a Tag object, and the ui.div() knows what to do with them. However, some functions can accept other kinds of objects. For example, a layout_sidebar() expects its first argument to be a Sidebar object.

The way that RecallContextManager is currently written, it simply accumulates all of the values from evaluating each line (with some exceptions, as noted above), and then passes them to the function when the with block exits -- it doesn't look at each value.

It might make sense to implement some sort of checking of the values before passing them to the function, but I think it would have to be customized for each function -- ui.div(), for example, accepts different kinds of values from ui.layout_sidebar(). And of course, a helpful error message is important.

@gadenbuie
Copy link
Collaborator

gadenbuie commented Nov 14, 2024

@wch With a small change (see #1773), we could simply not accumulate objects that aren't strings, simple scalars, or things that are convertible to renderable objects. It would make the example in this thread work out of the box without work-arounds. I also think it ends up being a bit more in line with what you'd expect in Express. The PR is in draft but all of our tests pass; feel free to check it out and leave thoughts there if you have time.

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

No branches or pull requests

2 participants