From 027635b978d2999ca128fe0462250a66915d6e60 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 12 Jan 2023 17:45:36 +0000 Subject: [PATCH] merge --- CHANGELOG.md | 1 + docs/index.md | 1 - src/textual/app.py | 6 +- src/textual/dom.py | 1 + src/textual/message_pump.py | 3 + src/textual/reactive.py | 72 +++++++---- .../__snapshots__/test_snapshots.ambr | 120 +++++++++--------- tests/test_reactive.py | 45 +++++-- 8 files changed, 153 insertions(+), 96 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed97fda10..71f5a6049 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added Widget._refresh_scroll to avoid expensive layout when scrolling https://github.com/Textualize/textual/pull/1524 - `events.Paste` now bubbles https://github.com/Textualize/textual/issues/1434 - Clock color in the `Header` widget now matches the header color https://github.com/Textualize/textual/issues/1459 +- Watch methods may now take no parameters ### Fixed diff --git a/docs/index.md b/docs/index.md index 584f60f93..1d776e820 100644 --- a/docs/index.md +++ b/docs/index.md @@ -1,4 +1,3 @@ - # Introduction Welcome to the [Textual](https://github.com/Textualize/textual) framework documentation. Built with ❤️ by [Textualize.io](https://www.textualize.io) diff --git a/src/textual/app.py b/src/textual/app.py index 89da6aaf6..2097abeeb 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -249,9 +249,9 @@ class App(Generic[ReturnType], DOMNode): Binding("shift+tab", "focus_previous", "Focus Previous", show=False), ] - title: Reactive[str] = Reactive("") - sub_title: Reactive[str] = Reactive("") - dark: Reactive[bool] = Reactive(True) + title: Reactive[str] = Reactive("", no_compute=True) + sub_title: Reactive[str] = Reactive("", no_compute=True) + dark: Reactive[bool] = Reactive(True, no_compute=True) def __init__( self, diff --git a/src/textual/dom.py b/src/textual/dom.py index 1204073a0..c1152e8b0 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -31,6 +31,7 @@ from .css.parse import parse_declarations from .css.styles import RenderStyles, Styles from .css.tokenize import IDENTIFIER from .message_pump import MessagePump +from .reactive import Reactive from .timer import Timer from .walk import walk_breadth_first, walk_depth_first diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index 5c3fca3f6..fef27b304 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -108,6 +108,7 @@ class MessagePump(metaclass=MessagePumpMeta): @property def is_running(self) -> bool: + """bool: Check if the message pump is running (potentially processing messages)""" return self._running @property @@ -326,6 +327,8 @@ class MessagePump(metaclass=MessagePumpMeta): try: await self._dispatch_message(events.Compose(sender=self)) await self._dispatch_message(events.Mount(sender=self)) + except Exception as error: + self.app._handle_exception(error) finally: # This is critical, mount may be waiting self._mounted_event.set() diff --git a/src/textual/reactive.py b/src/textual/reactive.py index d13039cde..3096ef83e 100644 --- a/src/textual/reactive.py +++ b/src/textual/reactive.py @@ -44,6 +44,7 @@ class Reactive(Generic[ReactiveType]): repaint (bool, optional): Perform a repaint on change. Defaults to True. init (bool, optional): Call watchers on initialize (post mount). Defaults to False. always_update (bool, optional): Call watchers even when the new value equals the old value. Defaults to False. + no_compute (bool, optional): Don't run compute methods when attribute is changed. Defaults to False. """ def __init__( @@ -54,12 +55,15 @@ class Reactive(Generic[ReactiveType]): repaint: bool = True, init: bool = False, always_update: bool = False, + no_compute: bool = False, ) -> None: self._default = default self._layout = layout self._repaint = repaint self._init = init self._always_update = always_update + self._no_compute = no_compute + self._is_compute = False @classmethod def init( @@ -69,6 +73,7 @@ class Reactive(Generic[ReactiveType]): layout: bool = False, repaint: bool = True, always_update: bool = False, + no_compute: bool = False, ) -> Reactive: """A reactive variable that calls watchers and compute on initialize (post mount). @@ -77,6 +82,7 @@ class Reactive(Generic[ReactiveType]): layout (bool, optional): Perform a layout on change. Defaults to False. repaint (bool, optional): Perform a repaint on change. Defaults to True. always_update (bool, optional): Call watchers even when the new value equals the old value. Defaults to False. + no_compute (bool, optional): Don't run compute methods when attribute is changed. Defaults to False. Returns: Reactive: A Reactive instance which calls watchers or initialize. @@ -87,6 +93,7 @@ class Reactive(Generic[ReactiveType]): repaint=repaint, init=True, always_update=always_update, + no_compute=no_compute, ) @classmethod @@ -105,24 +112,29 @@ class Reactive(Generic[ReactiveType]): return cls(default, layout=False, repaint=False, init=True) @classmethod - def _initialize_object(cls, obj: object) -> None: + def _initialize_object(cls, obj: Reactable) -> None: """Set defaults and call any watchers / computes for the first time. Args: obj (Reactable): An object with Reactive descriptors """ - if not hasattr(obj, "__reactive_initialized"): + return + if not getattr(obj, "__reactive_initialized", False): startswith = str.startswith + watchers = [] for key in obj.__class__.__dict__: if startswith(key, "_default_"): name = key[9:] + internal_name = f"_reactive_{name}" # Check defaults - if not hasattr(obj, name): + if internal_name not in obj.__dict__: # Attribute has no value yet default = getattr(obj, key) default_value = default() if callable(default) else default # Set the default vale (calls `__set__`) - setattr(obj, name, default_value) + obj.__dict__[internal_name] = default_value + watchers.append((name, default_value)) + setattr(obj, "__reactive_initialized", True) @classmethod @@ -140,6 +152,7 @@ class Reactive(Generic[ReactiveType]): # Check for compute method if hasattr(owner, f"compute_{name}"): # Compute methods are stored in a list called `__computes` + self._is_compute = True try: computes = getattr(owner, "__computes") except AttributeError: @@ -156,7 +169,13 @@ class Reactive(Generic[ReactiveType]): def __get__(self, obj: Reactable, obj_type: type[object]) -> ReactiveType: _rich_traceback_omit = True - value: _NotSet | ReactiveType = getattr(obj, self.internal_name, _NOT_SET) + + value: _NotSet | ReactiveType + if self._is_compute: + value = getattr(obj, f"compute_{self.name}")() + else: + value = getattr(obj, self.internal_name, _NOT_SET) + if isinstance(value, _NotSet): # No value present, we need to set the default init_name = f"_default_{self.name}" @@ -164,13 +183,19 @@ class Reactive(Generic[ReactiveType]): default_value = default() if callable(default) else default # Set and return the value setattr(obj, self.internal_name, default_value) + if self._init: + print("CHECK WATCHERS") self._check_watchers(obj, self.name, default_value) - return default_value + + if not self._no_compute: + self._compute(obj) + value = getattr(obj, self.internal_name) return value def __set__(self, obj: Reactable, value: ReactiveType) -> None: _rich_traceback_omit = True + # Reactive._initialize_object(obj) name = self.name current_value = getattr(obj, name) # Check for validate function @@ -182,8 +207,13 @@ class Reactive(Generic[ReactiveType]): if current_value != value or self._always_update: # Store the internal value setattr(obj, self.internal_name, value) + # Check all watchers self._check_watchers(obj, name, current_value) + + if not self._no_compute: + self._compute(obj) + # Refresh according to descriptor flags if self._layout or self._repaint: obj.refresh(repaint=self._repaint, layout=self._layout) @@ -225,10 +255,13 @@ class Reactive(Generic[ReactiveType]): bool: True if the watcher was run, or False if it was posted. """ _rich_traceback_omit = True - if count_parameters(watch_function) == 2: + param_count = count_parameters(watch_function) + if param_count == 2: watch_result = watch_function(old_value, value) - else: + elif param_count == 1: watch_result = watch_function(value) + else: + watch_result = watch_function() if isawaitable(watch_result): # Result is awaitable, so we need to await it within an async context obj.post_message_no_wait( @@ -244,24 +277,14 @@ class Reactive(Generic[ReactiveType]): require_compute = False watch_function = getattr(obj, f"watch_{name}", None) if callable(watch_function): - require_compute = require_compute or invoke_watcher( - watch_function, old_value, value - ) + invoke_watcher(watch_function, old_value, value) watchers: list[Callable] = getattr(obj, "__watchers", {}).get(name, []) for watcher in watchers: - require_compute = require_compute or invoke_watcher( - watcher, old_value, value - ) - - if require_compute: - # Run computes - obj.post_message_no_wait( - events.Callback(sender=obj, callback=partial(Reactive._compute, obj)) - ) + invoke_watcher(watcher, old_value, value) @classmethod - async def _compute(cls, obj: Reactable) -> None: + def _compute(cls, obj: Reactable) -> None: """Invoke all computes. Args: @@ -274,9 +297,8 @@ class Reactive(Generic[ReactiveType]): compute_method = getattr(obj, f"compute_{compute}") except AttributeError: continue - - value = await invoke(compute_method) - setattr(obj, compute, value) + value = compute_method() + setattr(obj, f"_reactive_{compute}", value) class reactive(Reactive[ReactiveType]): @@ -347,3 +369,5 @@ def watch( if init: current_value = getattr(obj, attribute_name, None) Reactive._check_watchers(obj, attribute_name, current_value) + print(obj, attribute_name) + print(watchers) diff --git a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr index 0f1da3cf2..14baa3354 100644 --- a/tests/snapshot_tests/__snapshots__/test_snapshots.ambr +++ b/tests/snapshot_tests/__snapshots__/test_snapshots.ambr @@ -13035,136 +13035,136 @@ font-weight: 700; } - .terminal-4137661484-matrix { + .terminal-3727479996-matrix { font-family: Fira Code, monospace; font-size: 20px; line-height: 24.4px; font-variant-east-asian: full-width; } - .terminal-4137661484-title { + .terminal-3727479996-title { font-size: 18px; font-weight: bold; font-family: arial; } - .terminal-4137661484-r1 { fill: #ffff00 } - .terminal-4137661484-r2 { fill: #c5c8c6 } - .terminal-4137661484-r3 { fill: #e3e3e3 } - .terminal-4137661484-r4 { fill: #ddeedd } - .terminal-4137661484-r5 { fill: #dde8f3;font-weight: bold } - .terminal-4137661484-r6 { fill: #ddedf9 } + .terminal-3727479996-r1 { fill: #ffff00 } + .terminal-3727479996-r2 { fill: #e3e3e3 } + .terminal-3727479996-r3 { fill: #c5c8c6 } + .terminal-3727479996-r4 { fill: #ddeedd } + .terminal-3727479996-r5 { fill: #dde8f3;font-weight: bold } + .terminal-3727479996-r6 { fill: #ddedf9 } - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - Layers + Layers - - - - ┌──────────────────────────────────┐ - ││It's full of stars! My God! It's full of sta - ││ - │This should float over the top│ - ││ - ││ - └──────────────────────────────────┘ - - - - - - - - - - - - - - - - -  T  Toggle Screen  + + + + ┌──────────────────────────────────┐Layers + ││It's full of stars! My God! It's full of sta + ││ + │This should float over the top│ + ││ + ││ + └──────────────────────────────────┘ + + + + + + + + + + + + + + + + +  T  Toggle Screen  diff --git a/tests/test_reactive.py b/tests/test_reactive.py index 5824f26f4..e3e30f698 100644 --- a/tests/test_reactive.py +++ b/tests/test_reactive.py @@ -2,8 +2,9 @@ import asyncio import pytest -from textual.app import App +from textual.app import App, ComposeResult from textual.reactive import reactive, var +from textual.widget import Widget OLD_VALUE = 5_000 NEW_VALUE = 1_000_000 @@ -81,7 +82,8 @@ async def test_watch_async_init_true(): await asyncio.wait_for(app.watcher_called_event.wait(), timeout=0.05) except TimeoutError: pytest.fail( - "Async watcher wasn't called within timeout when reactive init = True") + "Async watcher wasn't called within timeout when reactive init = True" + ) assert app.count == OLD_VALUE assert app.watcher_old_value == OLD_VALUE @@ -171,8 +173,12 @@ async def test_reactive_with_callable_default(): app = ReactiveCallable() async with app.run_test(): - assert app.value == OLD_VALUE # The value should be set to the return val of the callable - assert called_with_app is app # Ensure the App is passed into the reactive default callable + assert ( + app.value == OLD_VALUE + ) # The value should be set to the return val of the callable + assert ( + called_with_app is app + ) # Ensure the App is passed into the reactive default callable assert app.watcher_called_with == OLD_VALUE @@ -216,8 +222,7 @@ async def test_validate_init_true_set_before_dom_ready(): assert validator_call_count == 1 - -@pytest.mark.xfail(reason="Compute methods not called when init=True [issue#1227]") +# @pytest.mark.xfail(reason="Compute methods not called when init=True [issue#1227]") async def test_reactive_compute_first_time_set(): class ReactiveComputeFirstTimeSet(App): number = reactive(1) @@ -228,11 +233,10 @@ async def test_reactive_compute_first_time_set(): app = ReactiveComputeFirstTimeSet() async with app.run_test(): - await asyncio.sleep(.2) # TODO: We sleep here while issue#1218 is open + await asyncio.sleep(0.2) # TODO: We sleep here while issue#1218 is open assert app.double_number == 2 -@pytest.mark.xfail(reason="Compute methods not called immediately [issue#1218]") async def test_reactive_method_call_order(): class CallOrder(App): count = reactive(OLD_VALUE, init=False) @@ -266,3 +270,28 @@ async def test_reactive_method_call_order(): ] assert app.count == NEW_VALUE + 1 assert app.count_times_ten == (NEW_VALUE + 1) * 10 + + +async def test_premature_reactive_call(): + + watcher_called = False + + class BrokenWidget(Widget): + foo = reactive(1) + + def __init__(self) -> None: + super().__init__() + self.foo = "bar" + + async def watch_foo(self) -> None: + nonlocal watcher_called + watcher_called = True + + class PrematureApp(App): + def compose(self) -> ComposeResult: + yield BrokenWidget() + + app = PrematureApp() + async with app.run_test() as pilot: + assert watcher_called + app.exit()