Skip to content
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

Private validate compute #2708

Merged
merged 11 commits into from
Jun 6, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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

### Changed
Expand Down
15 changes: 14 additions & 1 deletion src/textual/message_pump.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -75,6 +75,19 @@ def __new__(
f"on_{namespace}_{camel_to_snake(value.__name__)}"
)

# Look for reactives with public AND private compute methods.
for attr_name, value in class_dict.items():
if attr_name.startswith("compute_") and callable(value):
reactive_name = attr_name[len("compute_") :]
rodrigogiraoserrao marked this conversation as resolved.
Show resolved Hide resolved
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

Expand Down
34 changes: 24 additions & 10 deletions src/textual/reactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 = (
Expand Down Expand Up @@ -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")
Expand All @@ -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)

Expand Down Expand Up @@ -184,11 +193,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
Expand Down Expand Up @@ -283,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)
)
Expand Down
72 changes: 55 additions & 17 deletions tests/test_reactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""

Expand All @@ -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