diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index fd3ec90f..2c09c46c 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -4,5 +4,4 @@ ### Experimental -- A new predicate, `OnlyIfPrevious`, to `filter()` messages based on the previous message. -- A new special case of `OnlyIfPrevious`, `ChangedOnly`, to skip messages if they are equal to the previous message. +- A new composable predicate, `WithPrevious`, to filter messages based on the previous message. diff --git a/src/frequenz/channels/experimental/__init__.py b/src/frequenz/channels/experimental/__init__.py index 6fe6b0d0..940cc304 100644 --- a/src/frequenz/channels/experimental/__init__.py +++ b/src/frequenz/channels/experimental/__init__.py @@ -10,12 +10,11 @@ """ from ._pipe import Pipe -from ._predicates import ChangedOnly, OnlyIfPrevious from ._relay_sender import RelaySender +from ._with_previous import WithPrevious __all__ = [ - "ChangedOnly", - "OnlyIfPrevious", + "WithPrevious", "Pipe", "RelaySender", ] diff --git a/src/frequenz/channels/experimental/_predicates.py b/src/frequenz/channels/experimental/_with_previous.py similarity index 52% rename from src/frequenz/channels/experimental/_predicates.py rename to src/frequenz/channels/experimental/_with_previous.py index bf5e45f2..90ab5a55 100644 --- a/src/frequenz/channels/experimental/_predicates.py +++ b/src/frequenz/channels/experimental/_with_previous.py @@ -1,7 +1,7 @@ # License: MIT # Copyright © 2024 Frequenz Energy-as-a-Service GmbH -"""Predicates to be used in conjuntion with `Receiver.filter()`.""" +"""Composable predicate to cache and compare with the previous message.""" from typing import Callable, Final, Generic, TypeGuard @@ -20,48 +20,43 @@ def __str__(self) -> str: _SENTINEL: Final[_Sentinel] = _Sentinel() -class OnlyIfPrevious(Generic[ChannelMessageT]): - """A predicate to check if a message has a particular relationship with the previous one. +class WithPrevious(Generic[ChannelMessageT]): + """A composable predicate to build predicates that can use also the previous message. - This predicate can be used to filter out messages based on a custom condition on the + This predicate can be used to filter messages based on a custom condition on the previous and current messages. This can be useful in cases where you want to process messages only if they satisfy a particular condition with respect to the previous message. - Tip: - If you want to use `==` as predicate, you can use the - [`ChangedOnly`][frequenz.channels.experimental.ChangedOnly] predicate. - - Example: Receiving only messages that are not the same instance as the previous one. + Example: Receiving only messages that are different from the previous one. ```python from frequenz.channels import Broadcast - from frequenz.channels.experimental import OnlyIfPrevious + from frequenz.channels.experimental import WithPrevious - channel = Broadcast[int | bool](name="example") - receiver = channel.new_receiver().filter(OnlyIfPrevious(lambda old, new: old is not new)) + channel = Broadcast[int](name="example") + receiver = channel.new_receiver().filter(WithPrevious(lambda old, new: old != new)) sender = channel.new_sender() # This message will be received as it is the first message. await sender.send(1) assert await receiver.receive() == 1 - # This message will be skipped as it is the same instance as the previous one. + # This message will be skipped as it equals to the previous one. await sender.send(1) - # This message will be received as it is a different instance from the previous - # one. - await sender.send(True) - assert await receiver.receive() is True + # This message will be received as it is a different from the previous one. + await sender.send(0) + assert await receiver.receive() == 0 ``` Example: Receiving only messages if they are bigger than the previous one. ```python from frequenz.channels import Broadcast - from frequenz.channels.experimental import OnlyIfPrevious + from frequenz.channels.experimental import WithPrevious channel = Broadcast[int](name="example") receiver = channel.new_receiver().filter( - OnlyIfPrevious(lambda old, new: new > old, first_is_true=False) + WithPrevious(lambda old, new: new > old, first_is_true=False) ) sender = channel.new_sender() @@ -90,6 +85,7 @@ class OnlyIfPrevious(Generic[ChannelMessageT]): def __init__( self, predicate: Callable[[ChannelMessageT, ChannelMessageT], bool], + /, *, first_is_true: bool = True, ) -> None: @@ -127,58 +123,3 @@ def __str__(self) -> str: def __repr__(self) -> str: """Return a string representation of this instance.""" return f"<{type(self).__name__}: {self._predicate!r} first_is_true={self._first_is_true!r}>" - - -class ChangedOnly(OnlyIfPrevious[object]): - """A predicate to check if a message is different from the previous one. - - This predicate can be used to filter out messages that are the same as the previous - one. This can be useful in cases where you want to avoid processing duplicate - messages. - - Warning: - This predicate uses the `!=` operator to compare messages, which includes all - the weirdnesses of Python's equality comparison (e.g., `1 == 1.0`, `True == 1`, - `True == 1.0`, `False == 0` are all `True`). - - If you need to use a different comparison, you can create a custom predicate - using [`OnlyIfPrevious`][frequenz.channels.experimental.OnlyIfPrevious]. - - Example: - ```python - from frequenz.channels import Broadcast - from frequenz.channels.experimental import ChangedOnly - - channel = Broadcast[int](name="skip_duplicates_test") - receiver = channel.new_receiver().filter(ChangedOnly()) - sender = channel.new_sender() - - # This message will be received as it is the first message. - await sender.send(1) - assert await receiver.receive() == 1 - - # This message will be skipped as it is the same as the previous one. - await sender.send(1) - - # This message will be received as it is different from the previous one. - await sender.send(2) - assert await receiver.receive() == 2 - ``` - """ - - def __init__(self, *, first_is_true: bool = True) -> None: - """Initialize this instance. - - Args: - first_is_true: Whether the first message should be considered as different - from the previous one. Defaults to `True`. - """ - super().__init__(lambda old, new: old != new, first_is_true=first_is_true) - - def __str__(self) -> str: - """Return a string representation of this instance.""" - return f"{type(self).__name__}" - - def __repr__(self) -> str: - """Return a string representation of this instance.""" - return f"{type(self).__name__}(first_is_true={self._first_is_true!r})" diff --git a/tests/experimental/test_predicates_changed_only.py b/tests/experimental/test_predicates_changed_only.py deleted file mode 100644 index 4d735d85..00000000 --- a/tests/experimental/test_predicates_changed_only.py +++ /dev/null @@ -1,121 +0,0 @@ -# License: MIT -# Copyright © 2024 Frequenz Energy-as-a-Service GmbH - -"""Tests for the ChangedOnly implementation. - -Most testing is done in the OnlyIfPrevious tests, these tests are limited to the -specifics of the ChangedOnly implementation. -""" - -from dataclasses import dataclass -from unittest.mock import MagicMock - -import pytest - -from frequenz.channels.experimental import ChangedOnly, OnlyIfPrevious - - -@dataclass(frozen=True, kw_only=True) -class EqualityTestCase: - """Test case for testing ChangedOnly behavior with tricky equality cases.""" - - title: str - first_value: object - second_value: object - expected_second_result: bool - - -EQUALITY_TEST_CASES = [ - # Python's equality weirdness cases - EqualityTestCase( - title="Integer equals float", - first_value=1, - second_value=1.0, - expected_second_result=False, - ), - EqualityTestCase( - title="Boolean equals integer", - first_value=True, - second_value=1, - expected_second_result=False, - ), - EqualityTestCase( - title="Boolean equals float", - first_value=True, - second_value=1.0, - expected_second_result=False, - ), - EqualityTestCase( - title="False equals zero", - first_value=False, - second_value=0, - expected_second_result=False, - ), - EqualityTestCase( - title="Zero equals False", - first_value=0, - second_value=False, - expected_second_result=False, - ), - # Edge cases that should be different - EqualityTestCase( - title="NaN is never equal to NaN", - first_value=float("nan"), - second_value=float("nan"), - expected_second_result=True, - ), - EqualityTestCase( - title="Different list instances with same content", - first_value=[1], - second_value=[1], - expected_second_result=False, - ), -] - - -def test_changed_only_inheritance() -> None: - """Test that ChangedOnly is properly inheriting from OnlyIfPrevious.""" - changed_only = ChangedOnly() - assert isinstance(changed_only, OnlyIfPrevious) - - -def test_changed_only_predicate_implementation() -> None: - """Test that ChangedOnly properly implements the inequality predicate.""" - # Create mock objects that we can control the equality comparison for - old = MagicMock() - new = MagicMock() - - # Set up the inequality comparison - # mypy doesn't understand mocking __ne__ very well - old.__ne__.return_value = True # type: ignore[attr-defined] - - changed_only = ChangedOnly() - # Skip the first message as it's handled by first_is_true - changed_only(old) - changed_only(new) - - # Verify that __ne__ was called with the correct argument - old.__ne__.assert_called_once_with(new) # type: ignore[attr-defined] - - -@pytest.mark.parametrize( - "test_case", - EQUALITY_TEST_CASES, - ids=lambda test_case: test_case.title, -) -def test_changed_only_equality_cases(test_case: EqualityTestCase) -> None: - """Test ChangedOnly behavior with Python's tricky equality cases. - - Args: - test_case: The test case containing the input values and expected result. - """ - changed_only = ChangedOnly() - assert changed_only(test_case.first_value) is True # First is always True - assert changed_only(test_case.second_value) is test_case.expected_second_result - - -def test_changed_only_representation() -> None: - """Test the string representation of ChangedOnly.""" - changed_only = ChangedOnly() - assert str(changed_only) == "ChangedOnly" - assert repr(changed_only) == "ChangedOnly(first_is_true=True)" diff --git a/tests/experimental/test_predicates_only_if_previous.py b/tests/experimental/test_with_previous.py similarity index 83% rename from tests/experimental/test_predicates_only_if_previous.py rename to tests/experimental/test_with_previous.py index b76bb96d..6c720759 100644 --- a/tests/experimental/test_predicates_only_if_previous.py +++ b/tests/experimental/test_with_previous.py @@ -1,21 +1,21 @@ # License: MIT # Copyright © 2024 Frequenz Energy-as-a-Service GmbH -"""Tests for the OnlyIfPrevious implementation.""" +"""Tests for the WithPrevious implementation.""" from dataclasses import dataclass from typing import Callable, Generic, TypeVar import pytest -from frequenz.channels.experimental import OnlyIfPrevious +from frequenz.channels.experimental import WithPrevious _T = TypeVar("_T") @dataclass(frozen=True, kw_only=True) class PredicateTestCase(Generic[_T]): - """Test case for testing OnlyIfPrevious behavior with different predicates.""" + """Test case for testing WithPrevious behavior with different predicates.""" title: str messages: list[_T] @@ -113,12 +113,12 @@ def is_not_same_instance(old: object, new: object) -> bool: ids=lambda test_case: test_case.title, ) def test_only_if_previous(test_case: PredicateTestCase[_T]) -> None: - """Test the OnlyIfPrevious with different predicates and sequences. + """Test the WithPrevious with different predicates and sequences. Args: test_case: The test case containing the input values and expected results. """ - only_if_previous = OnlyIfPrevious( + only_if_previous = WithPrevious( test_case.predicate, first_is_true=test_case.first_is_true, ) @@ -127,9 +127,9 @@ def test_only_if_previous(test_case: PredicateTestCase[_T]) -> None: def test_only_if_previous_state_independence() -> None: - """Test that multiple OnlyIfPrevious instances maintain independent state.""" - only_if_previous1 = OnlyIfPrevious(is_greater) - only_if_previous2 = OnlyIfPrevious(is_greater) + """Test that multiple WithPrevious instances maintain independent state.""" + only_if_previous1 = WithPrevious(is_greater) + only_if_previous2 = WithPrevious(is_greater) # First message should be accepted (first_is_true default is True) assert only_if_previous1(1) is True @@ -141,17 +141,17 @@ def test_only_if_previous_state_independence() -> None: def test_only_if_previous_str_representation() -> None: - """Test the string representation of OnlyIfPrevious.""" - only_if_previous = OnlyIfPrevious(is_greater) - assert str(only_if_previous) == "OnlyIfPrevious:is_greater" + """Test the string representation of WithPrevious.""" + only_if_previous = WithPrevious(is_greater) + assert str(only_if_previous) == "WithPrevious:is_greater" assert ( - repr(only_if_previous) == f"" + repr(only_if_previous) == f"" ) def test_only_if_previous_sentinel_str() -> None: """Test the string representation of the sentinel value.""" - only_if_previous = OnlyIfPrevious(always_true) + only_if_previous = WithPrevious(always_true) # Access the private attribute for testing purposes # pylint: disable=protected-access