Copilot commented on code in PR #169:
URL: https://github.com/apache/otava/pull/169#discussion_r3840585620
##########
otava/series.py:
##########
@@ -331,8 +358,8 @@ def append(self, time, new_data, attributes):
# r has a subset of all metrics, so can't just set change_points to r
for metric, cpglist in r.items():
self.change_points[metric] = cpglist
- self.weak_change_points = w
- self.change_points_by_time = self.change_points.by_time()
+ self.__weak_change_points = w
+ self.__change_points_by_time = self.change_points.by_time()
Review Comment:
`append()` recomputes and replaces the cached change-point results, but it
leaves `change_points_timestamp` at the time of the initial computation. After
any later append, serialized consumers therefore receive a stale calculation
timestamp. Refresh the timestamp alongside the other cached fields.
##########
tests/series_test.py:
##########
@@ -407,3 +408,83 @@ def test_series_raw_initialization():
assert len(series.time) == 3
assert series.data["throughput"] == [10.0, 12.0, 11.5]
+
+
+def test_change_points_computed_lazily_and_cached(monkeypatch):
+ from otava import series as series_module
+
+ calls = {"count": 0}
+ real_compute = series_module.compute_change_points
+
+ def counting_compute(*args, **kwargs):
+ calls["count"] += 1
+ return real_compute(*args, **kwargs)
+
+ monkeypatch.setattr(series_module, "compute_change_points",
counting_compute)
+
+ data = [1.0] * 10 + [5.0] * 10
+ test = Series(
+ "lazy_test",
+ branch=None,
+ time=list(range(len(data))),
+ metrics={"m1": Metric(1, 1.0), "m2": Metric(1, 1.0)},
+ data={"m1": data, "m2": data.copy()},
+ attributes={},
+ )
+
+ analyzed = test.analyze()
+ assert calls["count"] == 0
Review Comment:
Making `analyze()` lazy causes the existing
`test_change_point_detection_performance` loop at `tests/series_test.py:165` to
stop running change-point detection; it now times only object construction, so
detector performance regressions can no longer fail that test. The timed loop
should access the resulting `change_points` (or `change_points_by_time`) to
retain its original coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
