Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

NoMatches error occurring when changing screens quickly in version 0.49+ #4258

Closed
jakubziebin opened this issue Mar 5, 2024 · 14 comments
Closed

Comments

@jakubziebin
Copy link

Hello, I have a problem with a widget that has the role of data provider. The situation is as follows:

from __future__ import annotations

from random import randint
from typing import TypeVar, Generic

from textual import on, work
from textual.reactive import var
from textual.app import App, ComposeResult, Screen
from textual.binding import Binding
from textual.widget import Widget
from textual.widgets import Button, Static, Footer, TabbedContent, TabPane

ProviderT = TypeVar("ProviderT")


class BaseDataProvider(Widget, Generic[ProviderT]):
    _content: var[ProviderT] = var(None)

    def __init__(self) -> None:
        super().__init__()
        self.set_interval(1.5, self.update)

    @work
    async def update(self) -> None:
        """Define the logic to update the provider data."""

    @property
    def content(self) -> ProviderT:
        if self._content is None:
            return
        return self._content


class DataProvider(BaseDataProvider[int]):
    @work(name="Data update worker")
    async def update(self) -> None:
        number = randint(100, 1000)
        self._content = number


class AdditionalScreen(Screen[None]):
    example_var: var[int] = var(0)

    BINDINGS = [Binding("q", "quit_screen", "quit")]

    def __init__(self, number: int):
        super().__init__()
        self._number = number

    def on_mount(self) -> None:
        self.watch(self, "example_var", self._example)
        self.set_interval(1.5, self._example_set_var)

    def compose(self) -> ComposeResult:
        yield Static("TEST")
        yield Static(f"Actual number {self._number}")
        yield Footer()

    def _example_set_var(self) -> None:
        self.example_var = randint(1, 10)

    def _example(self) -> None:
        self.notify("EXAMPLE")

    def action_quit_screen(self) -> None:
        self.app.pop_screen()


class MyTabPane(TabPane):
    def compose(self) -> ComposeResult:
        yield Button("BUTTON")

    @on(Button.Pressed)
    def test(self) -> None:
        self.app.push_screen(AdditionalScreen(self.app.query_one(DataProvider).content))


class MyApp(App[None]):
    def compose(self) -> ComposeResult:
        with DataProvider():
            with TabbedContent():
                yield MyTabPane("TEST")


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

Sometimes when going in and out of AdditionalScreen quickly, NoMatches appears. I added another interval and watch on var to create artificial traffic, but when the application is larger the problem occurs very often.
The problem became more apparent after version 0.49, which prompted me to create this issue.

Video:
Screencast from 05.03.2024 14:11:20.webm

Textual diagnose:

# Textual Diagnostics

## Versions

| Name    | Value  |
|---------|--------|
| Textual | 0.48.2 |
| Rich    | 13.7.0 |

## Python

| Name           | Value                                                  |
|----------------|--------------------------------------------------------|
| Version        | 3.10.6                                                 |
| Implementation | CPython                                                |
| Compiler       | GCC 11.3.0                                             |
| Executable     | /home/dev/.pyenv/versions/3.10.6/envs/clive/bin/python |

## 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 | *Unknown*      |
| TERM                 | xterm-256color |
| COLORTERM            | truecolor      |
| FORCE_COLOR          | *Not set*      |
| NO_COLOR             | *Not set*      |

## Rich Console options

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

Copy link

github-actions bot commented Mar 5, 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

@mzebrak
Copy link

mzebrak commented Mar 5, 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.

@TomJGooding
Copy link
Contributor

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

@jakubziebin
Copy link
Author

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

@jakubziebin
Copy link
Author

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.

@mzebrak
Copy link

mzebrak commented Mar 6, 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.

@mzebrak
Copy link

mzebrak commented Mar 6, 2024

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

@davep
Copy link
Contributor

davep commented Mar 6, 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()

@mzebrak
Copy link

mzebrak commented Mar 6, 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.

@davep
Copy link
Contributor

davep commented Mar 6, 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.

@mzebrak
Copy link

mzebrak commented Mar 6, 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.

@willmcgugan
Copy link
Collaborator

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.

@mzebrak
Copy link

mzebrak commented Mar 7, 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.

@davep
Copy link
Contributor

davep commented Mar 7, 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.

@Textualize Textualize locked and limited conversation to collaborators Mar 7, 2024
@davep davep converted this issue into discussion #4268 Mar 7, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants