Giter Club home page Giter Club logo

Comments (14)

github-actions avatar github-actions commented on May 23, 2024

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

from textual.

mzebrak avatar mzebrak commented on May 23, 2024

I also experienced strange behavior with a newer version of Textual (was noticeable when I bumped the Textual version from 0.47.1 to 0.52.1). I think it may be related to changes in reactivity, because 0.49.0 adds data binding, but that's only my assumption. I think something is wrong with using self.query together close with screen managing (pop_screen?) methods and reactivity, because some race condition happens. In my case, NoMatches was observable in the textual's own widget - textual._header.Header.

The call stack looks like this:

  File "/app/app/__private/ui/app.py", line 278, in pop_screen
    self.title = f"{self.__class__.__name__} ({self.screen.__class__.__name__})"
  File "/home/mzebrak/.cache/pypoetry/virtualenvs/app-W0ktORSe-py3.10/lib/python3.10/site-packages/textual/reactive.py", line 276, in __set__
    self._check_watchers(obj, name, current_value)
  File "/home/mzebrak/.cache/pypoetry/virtualenvs/app-W0ktORSe-py3.10/lib/python3.10/site-packages/textual/reactive.py", line 319, in _check_watchers
    invoke_watcher(reactable, callback, old_value, value)
  File "/home/mzebrak/.cache/pypoetry/virtualenvs/app-W0ktORSe-py3.10/lib/python3.10/site-packages/textual/reactive.py", line 83, in invoke_watcher
    watch_result = cast(WatchCallbackNoArgsType, watch_function)()
  File "/home/mzebrak/.cache/pypoetry/virtualenvs/app-W0ktORSe-py3.10/lib/python3.10/site-packages/textual/widgets/_header.py", line 196, in set_title
    self.query_one(HeaderTitle).text = self.screen_title
  File "/home/mzebrak/.cache/pypoetry/virtualenvs/app-W0ktORSe-py3.10/lib/python3.10/site-packages/textual/dom.py", line 1260, in query_one
    return query.only_one() if expect_type is None else query.only_one(expect_type)
  File "/home/mzebrak/.cache/pypoetry/virtualenvs/app-W0ktORSe-py3.10/lib/python3.10/site-packages/textual/css/query.py", line 255, in only_one
    self.first(expect_type) if expect_type is not None else self.first()
  File "/home/mzebrak/.cache/pypoetry/virtualenvs/app-W0ktORSe-py3.10/lib/python3.10/site-packages/textual/css/query.py", line 226, in first
    raise NoMatches(f"No nodes match {self!r} on {self.node!r}")
textual.css.query.NoMatches: No nodes match <DOMQuery query='HeaderTitle'> on Header()

And it's because of the overridden pop_screen in my own App class like this:

def pop_screen(self) -> Screen[Any]:
    result = super().pop_screen()
    self.title = f"{self.__class__.__name__} ({self.screen.__class__.__name__})"
    return result

The flow is like this:

  • react to key pressed with BINDINGS and action_<> method
  • call pop_screen multiple times in a loop
  • when going through underlying screens - on some screen, the textual Header reports, that there is no HeaderTitle to be queried -> NoMatchesError

But this is very sporadic case, hard to reproduce, which I workarounded by overriding textual._header.Header class with:

    def __init__(self) -> None:
        self._header_title = HeaderTitle()
        super().__init__()

    def on_mount(self, event: Mount) -> None:
        # >>> start workaround for query_one(HeaderTitle) raising NoMatches error when title reactive is updated right
        # after pop_screen happens
        def set_title() -> None:
            self._header_title.text = self.screen_title

        def set_sub_title() -> None:
            self._header_title.sub_text = self.screen_sub_title

        event.prevent_default()

        self.watch(self.app, "title", set_title)
        self.watch(self.app, "sub_title", set_sub_title)
        self.watch(self.screen, "title", set_title)
        self.watch(self.screen, "sub_title", set_sub_title)
        # <<< end workaround

So I refer to the HeaderTitle by reference and not by using the self.query method. This seems to be a workaround for this issue because we omit the query system - since this workaround, no more No nodes match <DOMQuery query='HeaderTitle'> on Header() were observed.

from textual.

TomJGooding avatar TomJGooding commented on May 23, 2024

I can't reproduce this with the example code on v0.52.1. What am I missing?

from textual.

jakubziebin avatar jakubziebin commented on May 23, 2024

Hm, we have observed that it probably also depends on the machine, so on a slower one it may appear more often.
I reproduced it today without any problems:
Screencast from 06.03.2024 07:58:34.webm

from textual.

jakubziebin avatar jakubziebin commented on May 23, 2024

I have now tested this on versions 0.48.2 and 0.52.1, it appears less frequently on 0.52.1, but is still present.

from textual.

mzebrak avatar mzebrak commented on May 23, 2024

I can't reproduce this with the example code on v0.52.1. What am I missing?

I was able to create a more minimal example than the one presented.

Please take look at this:

from __future__ import annotations

from textual import on
from textual.app import App, ComposeResult, Screen
from textual.binding import Binding
from textual.widgets import Button, Footer, Label, Static


class SomeWidget(Label):
    pass


class SomeScreen(Screen[None]):
    BINDINGS = [Binding("q", "pop_screen", "Pop screen")]

    def __init__(self, desc: str) -> None:
        super().__init__()
        self.desc = desc

    def compose(self) -> ComposeResult:
        yield Static("Press q to pop_screen")
        yield Static(self.desc)
        yield Footer()


class MyApp(App[None]):
    def compose(self) -> ComposeResult:
        yield SomeWidget("Get this text via query_one")
        yield Button("Press me")

    @on(Button.Pressed)
    def test(self) -> None:
        self.app.push_screen(SomeScreen(self.app.query_one(SomeWidget).render()))


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

Nothing special here, seems to works just fine without additional stress to the CPU.

But when I stimulate some work on my CPU with e.g.:

stress --cpu 8 --timeout 600s --vm 8 --io 8 --vm-bytes 1024M 

I'm able to get this error:
Screencast from 06.03.2024 08:31:10.webm

The video is a bit laggy because my CPU is very loaded

But this issue is much better observable in more complex apps, e.g. it's easier to demonstrate when there's "something to do" like background tasks ran via set_interval or some reactivity watchers etc..

Textual diagnose

Textual Diagnostics

Versions

Name Value
Textual 0.52.1
Rich 13.7.1

Python

Name Value
Version 3.10.12
Implementation CPython
Compiler GCC 10.5.0
Executable /home/mzebrak/.pyenv/versions/3.10.12/envs/textual/bin/python3.10

Operating System

Name Value
System Linux
Release 6.5.0-21-generic
Version #21~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Feb 9 13:32:52 UTC 2

Terminal

Name Value
Terminal Application Terminator
TERM xterm-256color
COLORTERM truecolor
FORCE_COLOR Not set
NO_COLOR Not set

Rich Console options

Name Value
size width=143, height=26
legacy_windows False
min_width 1
max_width 143
is_terminal True
encoding utf-8
max_height 26
justify None
overflow None
no_wrap False
highlight None
markup None
height None

--- EDIT

I was also able to reproduce it on 0.47.1 and 0.42.0 with my MRE. But what's worth mentioning - it showed up in our application when the textual was bumped from 0.47.1 to 0.52.1. Also, a significant speedup of Textual was noticed with this bump, so I think that's related.

from textual.

mzebrak avatar mzebrak commented on May 23, 2024

I'm tagging @willmcgugan since IMO it's quite crucial

from textual.

davep avatar davep commented on May 23, 2024

Glancing at the examples shown here, on the surface, this seems like a variation of the problem where it's a bad idea to do work (especially queries) on your app, that assumes that App.screen is a specific (in this case the default) screen, when you have more than one screen involved in your app. This is often best illustrated with set_interval:

from textual.app import App, ComposeResult
from textual.reactive import var
from textual.screen import Screen
from textual.widgets import Label

class Child(Screen):

    BINDINGS = [
        ("escape", "pop_screen"),
    ]

    def compose(self) -> ComposeResult:
        yield Label("Press escape to go back")

class BadScreenApp(App[None]):

    BINDINGS = [
        ("s", "child_screen"),
    ]

    counter: var[int] = var(0)

    def compose(self) -> ComposeResult:
        yield Label("Press s for a child screen")
        yield Label(str(self.counter), id="counter")

    def count(self) -> None:
        self.counter += 1
        self.query_one("#counter", Label).update(str(self.counter))

    def on_mount(self) -> None:
        self.set_interval(0.25, self.count)

    def action_child_screen(self) -> None:
        self.push_screen(Child())

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

As you'll see, this works fine until the moment you press s, because then App.screen is not the default screen but the code is performing a query on the app, which will query whatever it thinks is the screen at that moment.

If you are going to have multiple screens it's almost always a good idea to not use the default screen, but instead do all your "main" work on some sort of main screen that you push from your app, then work from there. For example, a fixed version of the interval example:

from textual.app import App, ComposeResult
from textual.reactive import var
from textual.screen import Screen
from textual.widgets import Label

class Child(Screen):

    BINDINGS = [
        ("escape", "pop_screen"),
    ]

    def compose(self) -> ComposeResult:
        yield Label("Press escape to go back")

class Main(Screen):

    BINDINGS = [
        ("s", "child_screen"),
    ]

    counter: var[int] = var(0)

    def compose(self) -> ComposeResult:
        yield Label("Press s for a child screen")
        yield Label(str(self.counter), id="counter")

    def count(self) -> None:
        self.counter += 1
        self.query_one("#counter", Label).update(str(self.counter))

    def on_mount(self) -> None:
        self.set_interval(0.25, self.count)

    def action_child_screen(self) -> None:
        self.app.push_screen(Child())


class GoodScreenApp(App[None]):

    def on_mount(self) -> None:
        self.push_screen(Main())

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

from textual.

mzebrak avatar mzebrak commented on May 23, 2024

@davep For me, the example you included is unrelated to this issue - because it's more related to not destroying (stopping) the timer when it's defined on the main, default screen you mention, and another screen gets pushed meantime.

Mine MRE illustrates a little bit different thing - a race condition between push_screen/pop_screen and querying mechanism.
Looks like some widgets get destroyed (unmounted?) and the query takes place later than that. I think there is a lock missing - query cannot be done while mounting/unmounting some widgets.

Of course changing MRE to:

no default screen, same problem
from __future__ import annotations

from textual import on
from textual.app import App, ComposeResult, Screen
from textual.binding import Binding
from textual.widgets import Button, Footer, Label, Static


class SomeWidget(Label):
    pass


class SomeScreen(Screen[None]):
    BINDINGS = [Binding("q", "pop_screen", "Pop screen")]

    def __init__(self, desc: str) -> None:
        super().__init__()
        self.desc = desc

    def compose(self) -> ComposeResult:
        yield Static("Press q to pop_screen")
        yield Static(self.desc)
        yield Footer()


class MainScreen(Screen[None]):
    def compose(self) -> ComposeResult:
        yield SomeWidget("Get this text via query_one")
        yield Button("Press me")

    @on(Button.Pressed)
    def test(self) -> None:
        self.app.push_screen(SomeScreen(self.app.query_one(SomeWidget).render()))

class MyApp(App[None]):
    def on_mount(self) -> None:
        self.push_screen(MainScreen())


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

improves nothing.

-- edit
ps. In real application of course we don't use default screen, but have advanced screen stack with multiple screen variations (I think we got around 30 screens overall) and this issue is observable on them.

from textual.

davep avatar davep commented on May 23, 2024

Your improved MRE still has exactly the same problem, which is the related issue I was illustrating:

self.app.push_screen(SomeScreen(self.app.query_one(SomeWidget).render()))

As I understand it, this is the problematic line, right? This is the query that is failing? That code is querying whatever the current screen is, it isn't doing what I did in my fixed version.

from textual.

mzebrak avatar mzebrak commented on May 23, 2024

Oh ok, now I get you. Sorry I was focused on defining a new screen for it, and haven't noticed this difference - it goes like self.app.query in that case. Even though you mentioned it :

the code is performing a query on the app

So my MRE should do something like this instead:

self.app.push_screen(SomeScreen(self.query_one(SomeWidget).render()))

And yeah, this looks like is fixing this MRE but that's obvious because we only search within the scope of this widget.

Querying within the widget makes the most sense in this case, but I think it isn't enough to close this issue... It works differently when you do self.query in Widget and you do self.app.query which is unexpected and not quite developer-friendly.

In a real case - we use self.app.query to get something from the screen's parent.parent (so we don't have to pass it by reference very deep), but the solution of self.parent.parent.parent.parent.query_one does not satisfy me either.

Another thing is that this does not completely explain the situation explained above (fail with querying HeaderTitle). After all, in textual._header.Header you don't do self.app.query, but self.query within this widget.

from textual.

willmcgugan avatar willmcgugan commented on May 23, 2024

When you query from app, you will query from the currently active screen. If you have more than one screen then you may end up querying an unexpected screen (which was @davep 's point).

The solution is to use self.screen (from a widget), which will get the screen that the widget is attached to (and not whatever is current at the time). This appears to be the root cause of @jakubziebin and @mzebrak 's exception, and an easy fix.

With regards to the issue with the title. That seems to be caused by customizing pop_screen(), which is an odd thing to do. Handling ScreenResume would be more canonical.

from textual.

mzebrak avatar mzebrak commented on May 23, 2024

When you query from app, you will query from the currently active screen. If you have more than one screen then you may end up querying an unexpected screen (which was @davep 's point).

The solution is to use self.screen (from a widget), which will get the screen that the widget is attached to (and not whatever is current at the time). This appears to be the root cause of @jakubziebin and @mzebrak 's exception, and an easy fix.

With regards to the issue with the title. That seems to be caused by customizing pop_screen(), which is an odd thing to do. Handling ScreenResume would be more canonical.

Querying via screen instead of the app looks like is solving this issue. Thanks. But still, this is very unexpected and I think, at least should be mentioned somewhere in the docs.

With regards to the issue with the title. That seems to be caused by customizing pop_screen(), which is an odd thing to do. Handling ScreenResume would be more canonical.

Unfortunately seems like this cannot be done via ScreenResume as it does not bubble to the App.

See:

    def pop_screen(self) -> Screen[Any]:
        fun = super().pop_screen
        return self.__update_screen(lambda: fun())

    def __update_screen(self, callback: Callable[[], UpdateScreenResultT]) -> UpdateScreenResultT:
        """
        Auxiliary function to add some action on the default push_screen, switch_screen and pop_screen methods.

        Because of Textual's event ScreenResume not being bubbled up, we can't easily hook on it via
        `def on_screen_resume` so we have to override the push_screen, switch_screen and pop_screen methods.
        """
        reply = callback()
        self.title = f"{self.__class__.__name__} ({self.screen.__class__.__name__})"
        return reply

We've been doing this for a long time (I feel like pre 0.20.0 version), but with recent updates this started to be an issue.

from textual.

davep avatar davep commented on May 23, 2024

I'm moving this over to discussions as this doesn't seem to be about a resolvable issue; if some identifiable issue does come of that discussion we can open an issue from there.

from textual.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.