Textualize / textual

The lean application framework for Python. Build sophisticated user interfaces with a simple Python API. Run your apps in the terminal and a web browser.
https://textual.textualize.io/
MIT License
24.09k stars 742 forks source link

The type of dark attribute of class App might be inaccurate. (Reactive[bool] -> bool) #4614

Closed tongl2 closed 4 weeks ago

tongl2 commented 4 weeks ago

Current code in src/textual/app.py:

    dark: Reactive[bool] = Reactive(True, compute=False)

I personally believe the type of the attribute "dark" should be bool instead of Reactive. (Correct me if I was wrong.)

Reactive class does not implement __bool__ method, so Reactive(False) is actually True when casted to bool, which is quite confusing. In the code example, the first execution of self.app.dark = not self.app.dark is actually setting self.app.dark to boolean type False, which mean if I initially set self.app.dark to Reactive(False), the theme is still light unless I toggle it once. The code seems not using features of Reactive on dark attribute, so it will be more convincing changing the type to bool.

github-actions[bot] commented 4 weeks ago

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

tongl2 commented 4 weeks ago

Directly setting self.dark to False will raise a type-check lint: image

davep commented 4 weeks ago

Dark should be Reactive as it is a reactive. Moreover, setting dark to False has the desired effect with this code:

from textual.app import App, ComposeResult
from textual.widgets import Label

class LightApp(App[None]):

    def __init__(self):
        super().__init__()
        self.dark = False

    def compose(self) -> ComposeResult:
        yield Label("This is a light app")

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

I also see no type issues with the code (in this case using pyright):

Screenshot 2024-06-06 at 14 54 24

It might be a good idea to say what you're using for type checking, and perhaps which IDE you're using. For example, the type checker in (some versions of?) PyCharm are known to be lacking a little and sometimes can mislead people.

willmcgugan commented 4 weeks ago

Yeah, PyCharm doesn't understand descriptors all that well. @Haphaistos2333 feel free to report this to the PyCharm folks.

github-actions[bot] commented 4 weeks ago

Don't forget to star the repository!

Follow @textualizeio for Textual updates.