From 69ccf708916488ae03a076c265811272304e1fdc Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Tue, 29 Aug 2023 23:36:53 +0200 Subject: [PATCH] Rename datetime_to_index to to_internal_index The method is renamed to avoid confusion between indices in a window, which are passed by the user, and the internal indices that point to the actual position in the internal buffer. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- RELEASE_NOTES.md | 5 ++-- src/frequenz/sdk/timeseries/_moving_window.py | 2 +- .../timeseries/_periodic_feature_extractor.py | 2 +- .../sdk/timeseries/_ringbuffer/buffer.py | 30 +++++++++---------- tests/timeseries/test_ringbuffer.py | 4 +-- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index e52550fa3..0aed85df0 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -11,8 +11,9 @@ ## New Features - A tutorial section and a getting started tutorial - -- Remove `__setitem__` method from `OrderedRingBuffer` to enforce usage of the dedicated `update` method only. +- In `OrderedRingBuffer`: + - Rename `datetime_to_index` to `to_internal_index` to avoid confusion between the internal index and the external index. + - Remove `__setitem__` method to enforce usage of dedicated `update` method only. ## Bug Fixes diff --git a/src/frequenz/sdk/timeseries/_moving_window.py b/src/frequenz/sdk/timeseries/_moving_window.py index 69da7caf3..a72441920 100644 --- a/src/frequenz/sdk/timeseries/_moving_window.py +++ b/src/frequenz/sdk/timeseries/_moving_window.py @@ -384,7 +384,7 @@ def __getitem__(self, key: SupportsIndex | datetime | slice) -> float | ArrayLik return self._buffer[key] elif isinstance(key, datetime): _logger.debug("Returning value at time %s ", key) - return self._buffer[self._buffer.datetime_to_index(key)] + return self._buffer[self._buffer.to_internal_index(key)] elif isinstance(key, SupportsIndex): _logger.debug("Returning value at index %s ", key) return self._buffer[key] diff --git a/src/frequenz/sdk/timeseries/_periodic_feature_extractor.py b/src/frequenz/sdk/timeseries/_periodic_feature_extractor.py index e9c4c0c8a..2df7a3f9b 100644 --- a/src/frequenz/sdk/timeseries/_periodic_feature_extractor.py +++ b/src/frequenz/sdk/timeseries/_periodic_feature_extractor.py @@ -350,7 +350,7 @@ def _get_buffer_bounds( # add the offset to the oldest sample in the ringbuffer and wrap around # to get the start and end positions in the ringbuffer - rb_offset = self._buffer.datetime_to_index(self._buffer.time_bound_oldest) + rb_offset = self._buffer.to_internal_index(self._buffer.time_bound_oldest) start_pos = self._buffer.wrap(end_pos + self._period + rb_offset) end_pos = self._buffer.wrap(end_pos + rb_offset) diff --git a/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py b/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py index 3adb87e30..1d0c64e12 100644 --- a/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py +++ b/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py @@ -168,7 +168,7 @@ def update(self, sample: Sample[QuantityT]) -> None: value = sample.value.base_value else: value = np.nan - self._buffer[self.datetime_to_index(timestamp)] = value + self._buffer[self.to_internal_index(timestamp)] = value self._update_gaps(timestamp, prev_newest, not self.has_value(sample)) @@ -221,10 +221,12 @@ def newest_timestamp(self) -> datetime | None: return self.time_bound_newest - def datetime_to_index( + def to_internal_index( self, timestamp: datetime, allow_outside_range: bool = False ) -> int: - """Convert the given timestamp to an index. + """Convert the given timestamp to the position in the buffer. + + !!! Note: This method is meant for advanced use cases and should not generally be used. Args: timestamp: Timestamp to convert. @@ -258,10 +260,10 @@ def datetime_to_index( def window( self, start: datetime, end: datetime, *, force_copy: bool = True ) -> FloatArray: - """Request a view on the data between start timestamp and end timestamp. + """Request a copy or view on the data between start timestamp and end timestamp. - If the data is not used immediately it could be overwritten. Always request a copy if you keep the data around for longer. + Otherwise, if the data is not used immediately it could be overwritten. Will return a copy in the following cases: * The requested time period is crossing the start/end of the buffer. @@ -294,12 +296,10 @@ def window( if start == end: return np.array([]) if isinstance(self._buffer, np.ndarray) else [] - start_index = self.datetime_to_index(start) - end_index = self.datetime_to_index(end) + start_pos = self.to_internal_index(start) + end_pos = self.to_internal_index(end) - return self._wrapped_buffer_window( - self._buffer, start_index, end_index, force_copy - ) + return self._wrapped_buffer_window(self._buffer, start_pos, end_pos, force_copy) @staticmethod def _wrapped_buffer_window( @@ -562,10 +562,10 @@ def count_valid(self) -> int: ), ) - start_index = self.datetime_to_index(self._datetime_oldest) - end_index = self.datetime_to_index(self._datetime_newest) + start_pos = self.to_internal_index(self._datetime_oldest) + end_pos = self.to_internal_index(self._datetime_newest) - if end_index < start_index: - return len(self._buffer) - start_index + end_index + 1 - sum_missing_entries + if end_pos < start_pos: + return len(self._buffer) - start_pos + end_pos + 1 - sum_missing_entries - return end_index + 1 - start_index - sum_missing_entries + return end_pos + 1 - start_pos - sum_missing_entries diff --git a/tests/timeseries/test_ringbuffer.py b/tests/timeseries/test_ringbuffer.py index 5e36d0513..c50a4389c 100644 --- a/tests/timeseries/test_ringbuffer.py +++ b/tests/timeseries/test_ringbuffer.py @@ -182,9 +182,9 @@ def test_timestamp_ringbuffer_missing_parameter( ) == datetime(2, 2, 2, 0, 10, tzinfo=timezone.utc) buffer.update(Sample(datetime(2, 2, 2, 0, 7, 31, tzinfo=timezone.utc), Quantity(0))) - assert buffer.datetime_to_index( + assert buffer.to_internal_index( datetime(2, 2, 2, 0, 7, 31, tzinfo=timezone.utc) - ) == buffer.datetime_to_index(datetime(2, 2, 2, 0, 10, tzinfo=timezone.utc)) + ) == buffer.to_internal_index(datetime(2, 2, 2, 0, 10, tzinfo=timezone.utc)) assert len(buffer.gaps) == 2 # import pdb; pdb.set_trace()