Merge pull request #2708 from Textualize/private-validate-compute

Private validate compute

CHANGELOG.md

@@ -40,6 +40,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - `Input` has a new component class `input--suggestion` https://github.com/Textualize/textual/pull/2604
 - Added `Widget.remove_children` https://github.com/Textualize/textual/pull/2657
 - Added `Validator` framework and validation for `Input` https://github.com/Textualize/textual/pull/2600
+- Ability to have private and public validate methods https://github.com/Textualize/textual/pull/2708
+- Ability to have private compute methods https://github.com/Textualize/textual/pull/2708
 - Added `message_hook` to App.run_test https://github.com/Textualize/textual/pull/2702
 - Added `Sparkline` widget https://github.com/Textualize/textual/pull/2631
 

src/textual/message_pump.py

@@ -27,7 +27,7 @@
 from .errors import DuplicateKeyHandlers
 from .events import Event
 from .message import Message
-from .reactive import Reactive
+from .reactive import Reactive, TooManyComputesError
 from .timer import Timer, TimerCallback
 
 if TYPE_CHECKING:
@@ -75,6 +75,21 @@ def __new__(
                         f"on_{namespace}_{camel_to_snake(value.__name__)}"
                     )
 
+        # Look for reactives with public AND private compute methods.
+        prefix = "compute_"
+        prefix_len = len(prefix)
+        for attr_name, value in class_dict.items():
+            if attr_name.startswith(prefix) and callable(value):
+                reactive_name = attr_name[prefix_len:]
+                if (
+                    reactive_name in class_dict
+                    and isinstance(class_dict[reactive_name], Reactive)
+                    and f"_{attr_name}" in class_dict
+                ):
+                    raise TooManyComputesError(
+                        f"reactive {reactive_name!r} can't have two computes."
+                    )
+
         class_obj = super().__new__(cls, name, bases, class_dict, **kwargs)
         return class_obj
 

src/textual/reactive.py

@@ -32,6 +32,10 @@
 ReactiveType = TypeVar("ReactiveType")
 
 
+class TooManyComputesError(Exception):
+    """Raised when an attribute has public and private compute methods."""
+
+
 @rich.repr.auto
 class Reactive(Generic[ReactiveType]):
     """Reactive descriptor.
@@ -85,9 +89,9 @@ def _initialize_reactive(self, obj: Reactable, name: str) -> None:
             # Attribute already has a value
             return
 
-        compute_method = getattr(obj, f"compute_{name}", None)
+        compute_method = getattr(obj, self.compute_name, None)
         if compute_method is not None and self._init:
-            default = getattr(obj, f"compute_{name}")()
+            default = compute_method()
         else:
             default_or_callable = self._default
             default = (
@@ -122,7 +126,12 @@ def _reset_object(cls, obj: object) -> None:
 
     def __set_name__(self, owner: Type[MessageTarget], name: str) -> None:
         # Check for compute method
-        if hasattr(owner, f"compute_{name}"):
+        public_compute = f"compute_{name}"
+        private_compute = f"_compute_{name}"
+        compute_name = (
+            private_compute if hasattr(owner, private_compute) else public_compute
+        )
+        if hasattr(owner, compute_name):
             # Compute methods are stored in a list called `__computes`
             try:
                 computes = getattr(owner, "__computes")
@@ -135,7 +144,7 @@ def __set_name__(self, owner: Type[MessageTarget], name: str) -> None:
         self.name = name
         # The internal name where the attribute's value is stored
         self.internal_name = f"_reactive_{name}"
-        self.compute_name = f"compute_{name}"
+        self.compute_name = compute_name
         default = self._default
         setattr(owner, f"_default_{name}", default)
 
@@ -167,11 +176,13 @@ def __set__(self, obj: Reactable, value: ReactiveType) -> None:
 
         name = self.name
         current_value = getattr(obj, name)
-        # Check for validate function
-        validate_function = getattr(obj, f"validate_{name}", None)
-        # Call validate
-        if callable(validate_function):
-            value = validate_function(value)
+        # Check for private and public validate functions.
+        private_validate_function = getattr(obj, f"_validate_{name}", None)
+        if callable(private_validate_function):
+            value = private_validate_function(value)
+        public_validate_function = getattr(obj, f"validate_{name}", None)
+        if callable(public_validate_function):
+            value = public_validate_function(value)
         # If the value has changed, or this is the first time setting the value
         if current_value != value or self._always_update:
             # Store the internal value
@@ -266,7 +277,10 @@ def _compute(cls, obj: Reactable) -> None:
             try:
                 compute_method = getattr(obj, f"compute_{compute}")
             except AttributeError:
-                continue
+                try:
+                    compute_method = getattr(obj, f"_compute_{compute}")
+                except AttributeError:
+                    continue
             current_value = getattr(
                 obj, f"_reactive_{compute}", getattr(obj, f"_default_{compute}", None)
             )

tests/test_reactive.py

@@ -5,7 +5,7 @@
 import pytest
 
 from textual.app import App, ComposeResult
-from textual.reactive import Reactive, reactive, var
+from textual.reactive import Reactive, TooManyComputesError, reactive, var
 from textual.widget import Widget
 
 OLD_VALUE = 5_000
@@ -415,7 +415,21 @@ def _watch_counter(self) -> None:
         assert calls["public"] is True
 
 
-@pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2539")
+async def test_private_validate() -> None:
+    calls: dict[str, bool] = {"private": False}
+
+    class PrivateValidateTest(App):
+        counter = var(0, init=False)
+
+        def _validate_counter(self, _: int) -> None:
+            calls["private"] = True
+
+    async with PrivateValidateTest().run_test() as pilot:
+        assert calls["private"] is False
+        pilot.app.counter += 1
+        assert calls["private"] is True
+
+
 async def test_public_and_private_validate() -> None:
     """If a reactive/var has public and private validate both should get called."""
 
@@ -438,26 +452,50 @@ def _validate_counter(self, _: int) -> None:
         assert calls["public"] is True
 
 
-@pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2539")
+async def test_public_and_private_validate_order() -> None:
+    """The private validate should be called first."""
+
+    class ValidateOrderTest(App):
+        value = var(0, init=False)
+
+        def validate_value(self, value: int) -> int:
+            if value < 0:
+                return 42
+            return value
+
+        def _validate_value(self, value: int) -> int:
+            if value < 0:
+                return 73
+            return value
+
+    async with ValidateOrderTest().run_test() as pilot:
+        pilot.app.value = -10
+        assert pilot.app.value == 73
+
+
 async def test_public_and_private_compute() -> None:
     """If a reactive/var has public and private compute both should get called."""
 
-    calls: dict[str, bool] = {"private": False, "public": False}
+    with pytest.raises(TooManyComputesError):
 
-    class PrivateComputeTest(App):
-        counter = var(0, init=False)
+        class PublicAndPrivateComputeTest(App):
+            counter = var(0, init=False)
 
-        def compute_counter(self) -> int:
-            calls["public"] = True
-            return 23
+            def compute_counter(self):
+                pass
 
-        def _compute_counter(self) -> int:
-            calls["private"] = True
-            return 42
+            def _compute_counter(self):
+                pass
+
+
+async def test_private_compute() -> None:
+    class PrivateComputeTest(App):
+        double = var(0, init=False)
+        base = var(0, init=False)
+
+        def _compute_double(self) -> int:
+            return 2 * self.base
 
     async with PrivateComputeTest().run_test() as pilot:
-        assert calls["private"] is False
-        assert calls["public"] is False
-        _ = pilot.app.counter
-        assert calls["private"] is True
-        assert calls["public"] is True
+        pilot.app.base = 5
+        assert pilot.app.double == 10