Repository: frequenz-floss/frequenz-channels-python (language: python)

Issue:
Rename `OnlyIfPrevious` as `WithPrevious`
### What's needed?

Given #354, there is no more need to make it more clear if we are accepting messages for which the predicate returns `True` or `False`.

### Proposed solution

Rename to `WithPrevious`.

### Use cases

_No response_

### Alternatives and workarounds

_No response_

### Additional context

_No response_


Discussion / hints:

Fix patch:
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})"


Test patch:
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"<OnlyIfPrevious: {is_greater!r} first_is_true=True>"
+        repr(only_if_previous) == f"<WithPrevious: {is_greater!r} first_is_true=True>"
     )
 
 
 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