Skip to content

Commit e0af0af

Browse files
committed
Remove setitem magic from ring buffer
The method is missing wrapping of the index and therefore lacks the essential functionality that the ring buffer should provide. Instead, the `update` function should be used. So far this method is only used in a very specific unit test. Signed-off-by: cwasicki <[email protected]>
1 parent 2c8f61f commit e0af0af

File tree

3 files changed

+4
-57
lines changed

3 files changed

+4
-57
lines changed

RELEASE_NOTES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
<!-- Here goes the main new features and examples or instructions on how to use them -->
1414

15+
- Remove `__setitem__` method from `OrderedRingBuffer` to enforce usage of the dedicated `update` method only.
16+
1517
## Bug Fixes
1618

1719
<!-- Here goes notable bug fixes that are worth a special mention or explanation -->

src/frequenz/sdk/timeseries/_ringbuffer/buffer.py

Lines changed: 1 addition & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
"""Ringbuffer implementation with focus on time & memory efficiency."""
55

66

7-
from collections.abc import Iterable
87
from copy import deepcopy
98
from dataclasses import dataclass
109
from datetime import datetime, timedelta, timezone
11-
from typing import Generic, SupportsFloat, SupportsIndex, TypeVar, overload
10+
from typing import Generic, SupportsIndex, TypeVar, overload
1211

1312
import numpy as np
1413
import numpy.typing as npt
@@ -498,60 +497,6 @@ def wrap(self, index: int) -> int:
498497
"""
499498
return index % self.maxlen
500499

501-
@overload
502-
def __setitem__(self, index_or_slice: slice, value: Iterable[float]) -> None:
503-
"""Set values at the request slice positions.
504-
505-
No wrapping of the index will be done.
506-
Create a feature request if you require this function.
507-
508-
Args:
509-
index_or_slice: Slice specification of the requested data.
510-
value: Sequence of value to set at the given range.
511-
"""
512-
513-
@overload
514-
def __setitem__(self, index_or_slice: SupportsIndex, value: float) -> None:
515-
"""Set value at requested index.
516-
517-
No wrapping of the index will be done.
518-
Create a feature request if you require this function.
519-
520-
Args:
521-
index_or_slice: Index of the data.
522-
value: Value to set at the given position.
523-
"""
524-
525-
def __setitem__(
526-
self, index_or_slice: SupportsIndex | slice, value: float | Iterable[float]
527-
) -> None:
528-
"""Set item or slice at requested position.
529-
530-
No wrapping of the index will be done.
531-
Create a feature request if you require this function.
532-
533-
Args:
534-
index_or_slice: Index or slice specification of the requested data.
535-
value: Value to set at the given position.
536-
"""
537-
# There seem to be 2 different mypy bugs at play here.
538-
# First we need to check that the combination of input arguments are
539-
# correct to make the type checker happy (I guess it could be inferred
540-
# from the @overloads, but it's not currently working without this
541-
# hack).
542-
# Then we need to ignore a no-untyped-call error, for some reason it
543-
# can't get the type for self._buffer.__setitem__()
544-
if isinstance(index_or_slice, SupportsIndex) and isinstance(
545-
value, SupportsFloat
546-
):
547-
self._buffer.__setitem__(index_or_slice, value) # type: ignore[no-untyped-call]
548-
elif isinstance(index_or_slice, slice) and isinstance(value, Iterable):
549-
self._buffer.__setitem__(index_or_slice, value) # type: ignore[no-untyped-call]
550-
else:
551-
assert (
552-
False
553-
), f"Incompatible input arguments: {type(index_or_slice)=} {type(value)=}"
554-
555500
@overload
556501
def __getitem__(self, index_or_slice: SupportsIndex) -> float:
557502
"""Get item at requested position.

tests/timeseries/test_ringbuffer_serialization.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def load_dump_test(dumped: rb.OrderedRingBuffer[Any], path: str) -> None:
2929
# Fill with data so we have something to compare
3030
# Avoiding .update() because it takes very long for 40k entries
3131
for i in range(size):
32-
dumped[i] = i
32+
dumped._buffer[i] = i # pylint: disable=protected-access
3333

3434
# But use update a bit so the timestamp and gaps are initialized
3535
for i in range(0, size, 100):

0 commit comments

Comments
 (0)