From 1c7cb066aeed91d2e3a902c1575b19dfdb6299ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 30 May 2023 16:38:10 +0100 Subject: [PATCH 01/10] Activate private validate method test. --- tests/test_reactive.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_reactive.py b/tests/test_reactive.py index f0db5dcdf0..603fa22a3d 100644 --- a/tests/test_reactive.py +++ b/tests/test_reactive.py @@ -415,7 +415,6 @@ def _watch_counter(self) -> None: assert calls["public"] is True -@pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2539") async def test_public_and_private_validate() -> None: """If a reactive/var has public and private validate both should get called.""" From 648d5f43eee18feb11c78f5951026b76400729eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 30 May 2023 16:38:33 +0100 Subject: [PATCH 02/10] Use private validate methods. Private validate methods are checked before the public ones. --- src/textual/reactive.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/textual/reactive.py b/src/textual/reactive.py index 998d4079ba..0a9ff4a51b 100644 --- a/src/textual/reactive.py +++ b/src/textual/reactive.py @@ -184,11 +184,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 From 85722d61dbd872ec8f9d47d49bbe143e6af9826f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Wed, 31 May 2023 14:01:18 +0100 Subject: [PATCH 03/10] Test private validate methods. --- tests/test_reactive.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/test_reactive.py b/tests/test_reactive.py index 603fa22a3d..9c302d5205 100644 --- a/tests/test_reactive.py +++ b/tests/test_reactive.py @@ -415,6 +415,21 @@ def _watch_counter(self) -> None: assert calls["public"] is True +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.""" @@ -437,6 +452,27 @@ def _validate_counter(self, _: int) -> None: assert calls["public"] is True +async def test_public_and_private_validate_order() -> None: + """The private validate should be called first.""" + + class ValidateOrderTest(App): + value = reactive(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 + + @pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2539") async def test_public_and_private_compute() -> None: """If a reactive/var has public and private compute both should get called.""" From 41af804ca579178a1e37e44ace96996ffb957ec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Wed, 31 May 2023 16:09:22 +0100 Subject: [PATCH 04/10] Check for private AND public compute methods. We do this inside _MessagePumpMeta.__new__ because this runs at 'import time', and thus is essentially the earliest we can figure out if this is not going to work. --- src/textual/message_pump.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index e504af46ba..a6ac9fd176 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -30,7 +30,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: @@ -78,6 +78,19 @@ def __new__( f"on_{namespace}_{camel_to_snake(value.__name__)}" ) + # Look for reactives with public AND private compute methods. + for name, value in class_dict.items(): + if name.startswith("compute_") and callable(value): + reactive_name = name[len("compute_") :] + if ( + reactive_name in class_dict + and isinstance(class_dict[reactive_name], Reactive) + and f"_{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 From fdc7ba3f0b638b2218979cfae2158fcda2c07be4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Wed, 31 May 2023 16:17:06 +0100 Subject: [PATCH 05/10] Add tests for private computes. --- tests/test_reactive.py | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/tests/test_reactive.py b/tests/test_reactive.py index 9c302d5205..cb5a6b5f2f 100644 --- a/tests/test_reactive.py +++ b/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 @@ -456,7 +456,7 @@ async def test_public_and_private_validate_order() -> None: """The private validate should be called first.""" class ValidateOrderTest(App): - value = reactive(0, init=False) + value = var(0, init=False) def validate_value(self, value: int) -> int: if value < 0: @@ -473,26 +473,29 @@ def _validate_value(self, value: int) -> int: assert pilot.app.value == 73 -@pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2539") 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): + pass - def _compute_counter(self) -> int: - calls["private"] = True - return 42 + +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 From 2293f4041668641bdcae43b8fc493e0625ee5bae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Wed, 31 May 2023 16:17:15 +0100 Subject: [PATCH 06/10] Use private computes. --- src/textual/reactive.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/textual/reactive.py b/src/textual/reactive.py index 0a9ff4a51b..04ba313c13 100644 --- a/src/textual/reactive.py +++ b/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. @@ -102,9 +106,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 = ( @@ -139,7 +143,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") @@ -152,7 +161,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) @@ -285,7 +294,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) ) From 73019fa77371a87d0bf303dabf8f52456c76f689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Wed, 31 May 2023 17:50:23 +0100 Subject: [PATCH 07/10] Fix name shadowing issue. --- src/textual/message_pump.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index a6ac9fd176..94cd691b05 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -79,13 +79,13 @@ def __new__( ) # Look for reactives with public AND private compute methods. - for name, value in class_dict.items(): - if name.startswith("compute_") and callable(value): - reactive_name = name[len("compute_") :] + for attr_name, value in class_dict.items(): + if attr_name.startswith("compute_") and callable(value): + reactive_name = attr_name[len("compute_") :] if ( reactive_name in class_dict and isinstance(class_dict[reactive_name], Reactive) - and f"_{name}" in class_dict + and f"_{attr_name}" in class_dict ): raise TooManyComputesError( f"reactive {reactive_name!r} can't have two computes." From b1a69547893f2f59722617849a8bec8d8f0cf06e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Wed, 31 May 2023 17:52:11 +0100 Subject: [PATCH 08/10] Changelog. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe30a97786..c9175d66a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,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 ### Changed From 2057c463a406e7152d4b20cf7f60d7136d86715a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Mon, 5 Jun 2023 11:22:06 +0100 Subject: [PATCH 09/10] Extract prefix (length). --- src/textual/message_pump.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index a8e2807766..2a8678757b 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -76,9 +76,11 @@ def __new__( ) # Look for reactives with public AND private compute methods. + prefix = "compute_" + prefix_len = len("compute_") for attr_name, value in class_dict.items(): - if attr_name.startswith("compute_") and callable(value): - reactive_name = attr_name[len("compute_") :] + 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) From a26b0fd8e3778cc3b6a8fe3ad8c1153cd1857ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 6 Jun 2023 10:02:43 +0100 Subject: [PATCH 10/10] Update src/textual/message_pump.py --- src/textual/message_pump.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index 2a8678757b..2c191d3de7 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -77,7 +77,7 @@ def __new__( # Look for reactives with public AND private compute methods. prefix = "compute_" - prefix_len = len("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:]