Skip to content

Commit 036863a

Browse files
authored
Remove setitem magic from ring buffer (#670)
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.
2 parents b2f36cf + e0af0af commit 036863a

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
- A tutorial section and a getting started tutorial
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)