self.watch causes asynchronous callbacks to run synchronously

[mzebrak]

Might be related to: #3036

Correct me if I'm wrong, but it seems to me that the self.watch behavior included in the code snippet below which causes the asynchronous function passed to it to start working synchronously, is wrong. What I would expect is that all 3 DynamicLabels will be waiting at the same time and each of them will update after 1 second

from __future__ import annotations

import asyncio
from inspect import isawaitable
from time import perf_counter
from typing import TYPE_CHECKING, Any

from textual.app import App, ComposeResult
from textual.binding import Binding
from textual.reactive import var
from textual.widget import Widget
from textual.widgets import Footer, Label, LoadingIndicator

if TYPE_CHECKING:
    from collections.abc import Awaitable, Callable

    from textual.reactive import Reactable


class ReactiveHolder(Widget):
    DEFAULT_CSS = """
    ReactiveHolder {
        height: 0;
        width: 0;
    }
    """

    trigger = var(False, init=False)


class DynamicLabel(Widget):
    """A label that can be updated dynamically when a reactive variable changes."""

    DEFAULT_CSS = """
    DynamicLabel {
        height: auto;
    }

    DynamicLabel LoadingIndicator {
        height: 1;
    }
    """

    def __init__(
            self,
            obj_to_watch: Reactable,
            attribute_name: str,
            callback: Callable[[Any], str] | Callable[[Any], Awaitable[str]],
    ) -> None:
        super().__init__()

        self.__label = Label("loading...")
        self.__label.display = False

        self.__loading_indicator = LoadingIndicator()

        self.__obj_to_watch = obj_to_watch
        self.__attribute_name = attribute_name
        self.__callback = callback

    async def on_mount(self) -> None:
        self.watch(self.__obj_to_watch, self.__attribute_name, self.attribute_changed, init=False)

    def compose(self) -> ComposeResult:
        yield self.__loading_indicator
        yield self.__label

    async def attribute_changed(self, attribute: Any) -> None:
        await asyncio.sleep(1)  # Stimulate callback takes a long time to complete
        value = self.__callback(attribute)
        if isawaitable(value):
            value = await value
        self.__label.update(f"{value}")
        self.__loading_done()

    def __loading_done(self) -> None:
        if self.__loading_indicator.display:
            self.__loading_indicator.display = False
            self.__label.display = True


class MyApp(App):
    BINDINGS = [
        Binding("l", "load_data", "Load data"),
    ]

    def __init__(self) -> None:
        super().__init__()
        self.reactive_holder = ReactiveHolder()

    def on_mount(self) -> None:
        self.mount(self.reactive_holder)

    def action_load_data(self) -> None:
        self.button_pressed_time = perf_counter()
        self.reactive_holder.trigger = not self.reactive_holder.trigger

    def compose(self) -> ComposeResult:
        yield DynamicLabel(self.reactive_holder, "trigger", self.load_data)
        yield DynamicLabel(self.reactive_holder, "trigger", self.load_data)
        yield DynamicLabel(self.reactive_holder, "trigger", self.load_data)
        yield Footer()

    async def load_data(self, _: bool) -> str:
        return f"Loading took {perf_counter() - self.button_pressed_time:.2f} seconds"


if __name__ == "__main__":
    MyApp().run()

After pressing L, we can observe:

While the correct behaviour should be:

And it works like this if we wrap the callback in self.run_worker:

    async def on_mount(self) -> None:
        def delegate_work(attribute: Any) -> None:
            self.run_worker(self.attribute_changed(attribute))

        self.watch(self.__obj_to_watch, self.__attribute_name, delegate_work, init=False)

[github-actions[bot]]

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

This is an automated reply, generated by FAQtory

[mzebrak]

Additionally, maybe someone would have suggestions on how to implement such a DynamicLabel so as not to use composition, but inherit from Label? Because in this way we lose the ability to set CSS for DynamicLabel, e.g. text-align: center and we have to set it for DynamicLabel Label in the CSS along with width: 100% because content-align: center middle for DynamicLabel seems broken, it would somehow involve using the LoadingIndicator in def render instead of def compose, is it possible? Haven't seen any usage like this.

[willmcgugan]

What I would expect is that all 3 DynamicLabels will be waiting at the same time and each of them will update after 1 second

All callbacks are serialized with the widget's message queue. You almost always want this. If Textual launched tasks for such things, it would make it next to impossible to predict the state of your app from one call to the next. If you do want concurrency, you can use Textual's Workers.

Because in this way we lose the ability to set CSS for DynamicLabel, e.g. text-align: center and we have to set it for DynamicLabel Label

CSS types in Textual will work down the widget's MRO. So the CSS from the base class still applies.

n the CSS along with width: 100% because content-align: center middle for DynamicLabel seems broken, it would somehow involve using the LoadingIndicator in def render instead of def compose, is it possible? Haven't seen any usage like this.

I don't understand this. Perhaps you can rephrase?

[mzebrak]

So I have a periodically running background task (retrieving data from the API), which I store in some object, which is wrapped in the Textual reactive attribute (var). Now in many places, I want the data to be updated on the screen as well. DynamicLabel is a useful wrapper that allows me to achieve this without excessive code .

Previously, we used to display loading... in it when it was initialized, and now that the opportunity has arisen - we want to use the LoadingIndicator.

The usage looks like this:

def compose(self) -> ComposeResult:
    yield DynamicLabel(
              self.app.world,
              "profile_data",
              lambda profile_data: f"History entry: {humanize_datetime(profile_data.account.data.last_history_entry)}",
          )

[willmcgugan]

So you want a widget which either displays some content OR a loading indicator?

from textual.app import App, ComposeResult
from textual.widgets import Label, Footer, LoadingIndicator
from textual.widget import Widget
from textual.reactive import reactive


class LoadingLabel(Widget):

    DEFAULT_CSS = """
    LoadingLabel {
        height: auto;
        border: heavy magenta;
    }
    LoadingLabel Label {
        display: None;
    }

    LoadingLabel.-loaded Label {
        display: block;
    }

    LoadingLabel.-loaded LoadingIndicator {
	display: None;
    }

    """

    loaded = reactive(False)
    text = reactive("Your content here")

    def watch_loaded(self, loaded:bool) -> None:
        self.set_class(loaded, "-loaded")

    def compose(self) -> ComposeResult:
        yield Label(self.text)
        yield LoadingIndicator()


class LoadingApp(App):
    BINDINGS = [
	("l", "loaded", "Toggle loading")
    ]
    def compose(self) -> ComposeResult:
        yield LoadingLabel()
        yield Footer()


    def action_loaded(self) -> None:
        label = self.query_one(LoadingLabel)
        label.loaded = not label.loaded


LoadingApp().run()

[mzebrak]

More or less. I need a widget that can automatically update its renedarble in a dynamic way based on the reactive state change - exactly what I presented above, which will also display LoadingIndicator. It is true that such a DynamicLabel can internally use the LoadingLabel you presented instead of its own implementation of the mechanism for showing the LoadingIndicator, I like your implementation 😃 - but we still see the problem of increasing nesting here, and I would like to be able to just set text-align on DynamicLabel/LoadingLabel, and not play with align which will center the child component of Label.

[willmcgugan]

Nesting isn't really an issue. It's generally how you create dyanamic widgets with simpler atomic widgets. You can set text-align on the Label with CSS.

[mzebrak]

For me this is a kind of issue, because the earlier implementation of DynamicLabel relied on inheriting from Label - so you can say that DynamicLabel really was a Label. The introduction of the LoadingIndicator to it forces the resignation from inheriting from Label - which we have already established and forces a change in CSS - because I have to either refer to the more nested Label element, or change the way styles are defined in a given place.

Just like I actually handled that:

This is problematic - because once you have the styles ready, it's unlikely you'll want to come back to them every time you make changes to the Python code itself. And here you will probably say that it's inevitable 😛 because the restructuring forces it - yes, I agree, but sometimes we can try to maintain compatibility - and this is what I wanted to do, and what current capabilities prevent me from doing. That's why I wanted DynamicLabel to continue to inherit from Label (and to avoid changes to the current CSS), but this seems to be unavoidable after all.

And that's what I don't quite like about the textual - that some changes require again and again intervention in CSS, which is always quite a painful experience. No offense, of course there are a lot of other things that I like a lot, I'm just trying to outline a problem.