Skip to content

Commit

Permalink
Use at method in getitem magic of moving window
Browse files Browse the repository at this point in the history
The incomplete implementation of single element access in the getitem
magic is replaced by using the `at` method.

Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
  • Loading branch information
cwasicki committed Sep 27, 2023
1 parent 191dcdb commit ae9d080
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 14 deletions.
3 changes: 2 additions & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@
- In `OrderedRingBuffer` and `MovingWindow`:
- Support for integer indices is added.
- Add `count_covered` method to count the number of elements covered by the used time range.

- Add `at` method to `MovingWindow` to access a single element and use it in `__getitem__` magic to fully support single element access.



## Bug Fixes

- Fix rendering of diagrams in the documentation.
- The `__getitem__` magic of the `MovingWindow` is fixed to support the same functionality that the `window` method provides.
- Fixes incorrect implementation of single element access in `__getitem__` magic of `MovingWindow`.
10 changes: 3 additions & 7 deletions src/frequenz/sdk/timeseries/_moving_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,15 +416,11 @@ def __getitem__(self, key: SupportsIndex | datetime | slice) -> float | ArrayLik
raise ValueError("Slicing with a step other than 1 is not supported.")
return self.window(key.start, key.stop)

if self._buffer.count_valid() == 0:
raise IndexError("The buffer is empty.")

if isinstance(key, datetime):
_logger.debug("Returning value at time %s ", key)
return self._buffer[self._buffer.to_internal_index(key)]
return self.at(key)

if isinstance(key, SupportsIndex):
_logger.debug("Returning value at index %s ", key)
return self._buffer[key]
return self.at(key.__index__())

raise TypeError(
"Key has to be either a timestamp or an integer "
Expand Down
11 changes: 5 additions & 6 deletions tests/timeseries/test_moving_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ async def test_access_window_by_index() -> None:
window, sender = init_moving_window(timedelta(seconds=2))
async with window:
await push_logical_meter_data(sender, [1, 2, 3])
assert np.array_equal(window[0], 3.0) # bug: should be 2
assert np.array_equal(window[1], 2.0) # bug: should be 3
assert np.array_equal(window[-1], 2.0) # bug: should be 3
assert np.array_equal(window[-2], 3.0) # bug: should be 2
assert np.array_equal(window[0], 2.0)
assert np.array_equal(window[1], 3.0)
assert np.array_equal(window[-1], 3.0)
assert np.array_equal(window[-2], 2.0)
with pytest.raises(IndexError):
_ = window[3]
with pytest.raises(IndexError):
Expand All @@ -102,13 +102,12 @@ async def test_access_window_by_timestamp() -> None:
assert np.array_equal(window.at(dt(1)), 1.0)
assert np.array_equal(window[dt(2)], 2.0)
assert np.array_equal(window.at(dt(2)), 2.0)
assert np.array_equal(window[dt(3)], 1.0) # bug: should raise
with pytest.raises(IndexError):
_ = window[dt(0)]
with pytest.raises(IndexError):
_ = window.at(dt(0))
with pytest.raises(IndexError):
_ = window[dt(4)]
_ = window[dt(3)]
with pytest.raises(IndexError):
_ = window.at(dt(3))

Expand Down

0 comments on commit ae9d080

Please sign in to comment.