Skip to content

Commit eabd65c

Browse files
committed
Remove Sample comparison hack
The Sample object implemented comparisons by only comparing timestamps. This was only done to cope with a limitation of `bisect()` before Python 3.10 (where no key argument could be passed). Now that we are using Python 3.11 as the minimal version, we can remove that hack and make Samples compare using both the datetime and the value as one would expect. Signed-off-by: Leandro Lucarella <leandro.lucarella@frequenz.com>
1 parent 32a8d46 commit eabd65c

3 files changed

Lines changed: 10 additions & 11 deletions

File tree

RELEASE_NOTES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ This release drops support for Python versions older than 3.11.
1414

1515
* The `LogicalMeter` no longer takes a `component_graph` parameter.
1616

17+
* Now `frequenz.sdk.timeseries.Sample` uses a more sensible comparison. Before this release `Sample`s were compared only based on the `timestamp`. This was due to a limitation in Python versions earlier than 3.10. Now that the minimum supported version is 3.11 this hack is not needed anymore and `Sample`s are compared using both `timestamp` and `value` as most people probably expects.
18+
1719
## New Features
1820

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

src/frequenz/sdk/timeseries/_base_types.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@
1212
"""The UNIX epoch (in UTC)."""
1313

1414

15-
# Ordering by timestamp is a bit arbitrary, and it is not always what might be
16-
# wanted. We are using this order now because usually we need to do binary
17-
# searches on sequences of samples, and the Python `bisect` module doesn't
18-
# support providing a key until Python 3.10.
1915
@dataclass(frozen=True, order=True)
2016
class Sample:
2117
"""A measurement taken at a particular point in time.
@@ -25,10 +21,10 @@ class Sample:
2521
coherent view on a group of component metrics for a particular timestamp.
2622
"""
2723

28-
timestamp: datetime = field(compare=True)
24+
timestamp: datetime
2925
"""The time when this sample was generated."""
3026

31-
value: float | None = field(compare=False, default=None)
27+
value: float | None = None
3228
"""The value of this sample."""
3329

3430

src/frequenz/sdk/timeseries/_resampling.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -705,11 +705,12 @@ def resample(self, timestamp: datetime) -> Sample:
705705
)
706706
minimum_relevant_timestamp = timestamp - period * conf.max_data_age_in_periods
707707

708-
# We need to pass a dummy Sample to bisect because it only support
709-
# specifying a key extraction function in Python 3.10, so we need to
710-
# compare samples at the moment.
711-
min_index = bisect(self._buffer, Sample(minimum_relevant_timestamp, None))
712-
max_index = bisect(self._buffer, Sample(timestamp, None))
708+
min_index = bisect(
709+
self._buffer,
710+
minimum_relevant_timestamp,
711+
key=lambda s: s.timestamp,
712+
)
713+
max_index = bisect(self._buffer, timestamp, key=lambda s: s.timestamp)
713714
# Using itertools for slicing doesn't look very efficient, but
714715
# experiments with a custom (ring) buffer that can slice showed that
715716
# it is not that bad. See:

0 commit comments

Comments
 (0)