From 0a41680017366b68e3fc880f70d236dd7b830449 Mon Sep 17 00:00:00 2001
From: cwasicki <126617870+cwasicki@users.noreply.github.com>
Date: Tue, 5 Sep 2023 17:58:29 +0200
Subject: [PATCH 1/2] Add more unit tests for gaps

Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
---
 tests/timeseries/test_ringbuffer.py | 61 +++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/tests/timeseries/test_ringbuffer.py b/tests/timeseries/test_ringbuffer.py
index ee2344989..21ca901c0 100644
--- a/tests/timeseries/test_ringbuffer.py
+++ b/tests/timeseries/test_ringbuffer.py
@@ -192,6 +192,67 @@ def test_timestamp_ringbuffer_missing_parameter(
     assert len(buffer.gaps) == 1
 
 
+def dt(i: int) -> datetime:  # pylint: disable=invalid-name
+    """Create datetime objects from indices.
+
+    Args:
+        i: Index to create datetime from.
+
+    Returns:
+        Datetime object.
+    """
+    return datetime.fromtimestamp(i, tz=timezone.utc)
+
+
+def test_gaps() -> None:
+    """Test gap treatment in ordered ring buffer."""
+    buffer = OrderedRingBuffer([0.0] * 5, ONE_SECOND)
+    assert len(buffer) == 0
+    assert len(buffer.gaps) == 0
+
+    buffer.update(Sample(dt(0), Quantity(0)))
+    assert len(buffer) == 1
+    assert len(buffer.gaps) == 1
+
+    buffer.update(Sample(dt(6), Quantity(0)))
+    assert len(buffer) == 1
+    assert len(buffer.gaps) == 1
+
+    buffer.update(Sample(dt(2), Quantity(2)))
+    buffer.update(Sample(dt(3), Quantity(3)))
+    buffer.update(Sample(dt(4), Quantity(4)))
+    assert len(buffer) == 4
+    assert len(buffer.gaps) == 1
+
+    buffer.update(Sample(dt(3), None))
+    assert len(buffer) == 3
+    assert len(buffer.gaps) == 2
+
+    buffer.update(Sample(dt(3), Quantity(np.nan)))
+    assert len(buffer) == 4  # should be 3
+    assert len(buffer.gaps) == 1  # should be 2
+
+    buffer.update(Sample(dt(2), Quantity(np.nan)))
+    assert len(buffer) == 4  # should be 2
+    assert len(buffer.gaps) == 1  # should be 2
+
+    buffer.update(Sample(dt(3), Quantity(3)))
+    assert len(buffer) == 4  # should be 3
+    assert len(buffer.gaps) == 1  # should be 2
+
+    buffer.update(Sample(dt(2), Quantity(2)))
+    assert len(buffer) == 4
+    assert len(buffer.gaps) == 1
+
+    buffer.update(Sample(dt(5), Quantity(5)))
+    assert len(buffer) == 5
+    assert len(buffer.gaps) == 0
+
+    buffer.update(Sample(dt(99), None))
+    assert len(buffer) == 4  # bug: should be 0 (whole range gap)
+    assert len(buffer.gaps) == 1
+
+
 @pytest.mark.parametrize(
     "buffer",
     [

From c1c8828e45767a2ccc07c6dc7df6ed46940f71c5 Mon Sep 17 00:00:00 2001
From: cwasicki <126617870+cwasicki@users.noreply.github.com>
Date: Tue, 5 Sep 2023 17:00:30 +0200
Subject: [PATCH 2/2] Treat NaNs as missing values for gaps in ring buffer

Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
---
 RELEASE_NOTES.md                              |  1 +
 .../sdk/timeseries/_ringbuffer/buffer.py      | 19 +++++++++++++++++--
 tests/timeseries/test_ringbuffer.py           | 12 ++++++------
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 271656864..47b33b1f2 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -20,6 +20,7 @@
 
 - A new class `Fuse` has been added to represent fuses. This class has a member variable `max_current` which represents the maximum current that can course through the fuse. If the current flowing through a fuse is greater than this limit, then the fuse will break the circuit.
 
+- NaN values are treated as missing when gaps are determined in the `OrderedRingBuffer`.
 
 ## Bug Fixes
 
diff --git a/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py b/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py
index 44daa95cf..5e0c2721e 100644
--- a/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py
+++ b/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py
@@ -108,6 +108,17 @@ def gaps(self) -> List[Gap]:
         """
         return self._gaps
 
+    def has_value(self, sample: Sample[QuantityT]) -> bool:
+        """Check if a sample has a value and it's not NaN.
+
+        Args:
+            sample: sample to check.
+
+        Returns:
+            True if the sample has a value and it's not NaN.
+        """
+        return not (sample.value is None or sample.value.isnan())
+
     @property
     def maxlen(self) -> int:
         """Get the max length.
@@ -154,10 +165,14 @@ def update(self, sample: Sample[QuantityT]) -> None:
         )
 
         # Update data
-        value: float = np.nan if sample.value is None else sample.value.base_value
+        if self.has_value(sample):
+            assert sample.value is not None
+            value = sample.value.base_value
+        else:
+            value = np.nan
         self._buffer[self.datetime_to_index(timestamp)] = value
 
-        self._update_gaps(timestamp, prev_newest, sample.value is None)
+        self._update_gaps(timestamp, prev_newest, not self.has_value(sample))
 
     @property
     def time_bound_oldest(self) -> datetime:
diff --git a/tests/timeseries/test_ringbuffer.py b/tests/timeseries/test_ringbuffer.py
index 21ca901c0..11ddae4a3 100644
--- a/tests/timeseries/test_ringbuffer.py
+++ b/tests/timeseries/test_ringbuffer.py
@@ -229,16 +229,16 @@ def test_gaps() -> None:
     assert len(buffer.gaps) == 2
 
     buffer.update(Sample(dt(3), Quantity(np.nan)))
-    assert len(buffer) == 4  # should be 3
-    assert len(buffer.gaps) == 1  # should be 2
+    assert len(buffer) == 3
+    assert len(buffer.gaps) == 2
 
     buffer.update(Sample(dt(2), Quantity(np.nan)))
-    assert len(buffer) == 4  # should be 2
-    assert len(buffer.gaps) == 1  # should be 2
+    assert len(buffer) == 2
+    assert len(buffer.gaps) == 2
 
     buffer.update(Sample(dt(3), Quantity(3)))
-    assert len(buffer) == 4  # should be 3
-    assert len(buffer.gaps) == 1  # should be 2
+    assert len(buffer) == 3
+    assert len(buffer.gaps) == 2
 
     buffer.update(Sample(dt(2), Quantity(2)))
     assert len(buffer) == 4