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

Need stronger ui.hold()-like context manager for non-UI objects #1083

Open
jcheng5 opened this issue Jan 28, 2024 · 4 comments
Open

Need stronger ui.hold()-like context manager for non-UI objects #1083

jcheng5 opened this issue Jan 28, 2024 · 4 comments
Milestone

Comments

@jcheng5
Copy link
Collaborator

jcheng5 commented Jan 28, 2024

We need another context manager, maybe ui.discard(), to prevent Shiny Express from trying to interpret non-UI objects as UI. Or, perhaps Shiny Express should not be so strict about what kinds of objects are allowed to be displayhooked (no-op instead of error).

from shiny.express import ui

class A:
    def __init__(self):
        self.options = {}

    def set_option(self, name, value):
        self.options[name] = value
        
        # for method chaining purposes
        return self

a = A()

# This next line causes the error.
# Enclosing the line in `with ui.hold()` does not help.
a.set_option("host", "localhost")

with ui.card():
    "Hello"

For now the best workaround I can think of is to turn such lines into dummy assignment statements, like _ = a.set_option("host", "localhost"), but that is not very satisfying.

@jcheng5
Copy link
Collaborator Author

jcheng5 commented Jan 28, 2024

We could also have a more imperative "echo on/off" type of function.

ui.echo(False)

# Do lots of non-UI stuff

ui.echo(True)

You wouldn't even have to wrap it in try/finally, instead you could say that if there is an enclosing UI context manager (like ui.card()), the effect of ui.echo(False) does not survive beyond the end of that context manager.

The nice thing about this approach is not having to indent the non-UI stuff.

There's a good chance this is a terrible idea and I'm not thinking clearly about it, it's pretty late.

@wch
Copy link
Collaborator

wch commented Jan 28, 2024

The specific problem with ui.hold() is that it's a RecallContextManager that wraps a TagList, and the TagList cares about what goes into it. This is so all the things in the with ui.hold() as x: block can be referred to as a single object x. But now that I think about it, I think it could be changed to a regular list without breaking anything.

Or another alternative: the objects get put in a subclass of list, like HoldList. If a function consumes that object and doesn't know about HoldLists, then it gets treated like a list. But if function consumes it and and does know about HoldLists (like a RecallContextManager) then it can use the spread operator on it.

@wch
Copy link
Collaborator

wch commented Jan 28, 2024

Another thought that I had was that perhaps RecallContextManager could add a filter argument, so we could have something like:

def page_auto_cm(
    *,
    filter: Callable[[object], bool] | None = None,
) -> RecallContextManager[Tag]:
    return RecallContextManager(
        ui.page_auto,
        filter=filter,
    )

Where the filter returns True if the displayhooked objects should be stored by the RecallContextManager, False otherwise.

And then the default filter function could be one that returns True only for TagChild objects.

(Even with this, there would still be the issue of ui.hold() not accepting non-TagChild objects, because it doesn't use a RecallContextManager. But that could be solved by the proposal in my previous comment.)

@wch
Copy link
Collaborator

wch commented Jan 30, 2024

  • Create subclass of RecallContextManager, RecallContextManagerUi, with a default filter function.
  • Use a filter function that throws as soon as the displayhook is called, print error message with line number and text
    • Need to add contextvar state to track current line number.
  • ui.hold() should store contents with a list instead of TagList

@wch wch added this to the v0.8.0 milestone Feb 2, 2024
@wch wch modified the milestones: v0.8.0, v0.9.0 Feb 29, 2024
@schloerke schloerke modified the milestones: v0.9.0, v0.9.1 May 8, 2024
@schloerke schloerke modified the milestones: v0.10.0, v0.11.0 May 21, 2024
@wch wch modified the milestones: v1.0.0, v1.2.0 Jul 17, 2024
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

3 participants