This is an automated email from the ASF dual-hosted git repository. asorokoumov pushed a commit to branch fix-perf-test in repository https://gitbox.apache.org/repos/asf/otava.git
commit c5e1d399f488e8e25a9f7ad9e05179efca83dd52 Author: Alex Sorokoumov <[email protected]> AuthorDate: Wed Dec 17 23:23:55 2025 -0800 analysis.split sorts change points before getting intervals Prior to this patch, change points discovered while scanning sliding windows are added to the end of the list. At the same time, SignificanceTester.get_intervals expects change points to be sorted. As a result, intervals contain 1 or more None that cause ValueError in tester.change_point. --- otava/analysis.py | 2 ++ otava/change_point_divisive/base.py | 6 +++++- tests/change_point_divisive_test.py | 32 +++++++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/otava/analysis.py b/otava/analysis.py index 74f6144..5ff8975 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -271,6 +271,8 @@ def split(series: Sequence[SupportsFloat], window_len: int = 30, max_pvalue: flo if cp not in change_points: change_points += [cp] + # Sort change points by index; required by get_intervals() and maintained by merge() + change_points.sort(key=lambda cp: cp.index) intervals = tester.get_intervals(change_points) return [tester.change_point(cp.to_candidate(), series, intervals) for cp in change_points] diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index fbbf91b..edde8e8 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -68,7 +68,11 @@ class SignificanceTester(Generic[GenericStats]): self.max_pvalue = max_pvalue def get_intervals(self, change_points: List[ChangePoint[GenericStats]]) -> List[slice]: - '''Returns list of slices of the series''' + '''Returns list of slices of the series. Change points must be sorted by index.''' + assert all( + change_points[i].index <= change_points[i + 1].index + for i in range(len(change_points) - 1) + ), "Change points must be sorted by index" intervals = [ slice( 0 if i == 0 else change_points[i - 1].index, diff --git a/tests/change_point_divisive_test.py b/tests/change_point_divisive_test.py index 34765bb..45d4431 100644 --- a/tests/change_point_divisive_test.py +++ b/tests/change_point_divisive_test.py @@ -16,8 +16,10 @@ # under the License. import numpy as np +import pytest -from otava.analysis import TTestSignificanceTester +from otava.analysis import TTestSignificanceTester, TTestStats +from otava.change_point_divisive.base import ChangePoint from otava.change_point_divisive.calculator import PairDistanceCalculator from otava.change_point_divisive.detector import ChangePointDetector from otava.change_point_divisive.significance_test import PermutationsSignificanceTester @@ -113,3 +115,31 @@ def test_ttest(): cpd = ChangePointDetector(significance_tester=st, calculator=PairDistanceCalculator) cps = cpd.get_change_points(series=sequence) assert [cp.index for cp in cps] == CHANGE_POINTS_INDS + + +def test_get_intervals_requires_sorted_change_points(): + """Test that get_intervals() raises AssertionError when change points are not sorted by index.""" + tester = TTestSignificanceTester(max_pvalue=0.01) + stats = TTestStats(mean_1=1.0, mean_2=2.0, std_1=0.1, std_2=0.1, pvalue=0.001) + + # Sorted change points should work + sorted_cps = [ + ChangePoint(index=5, qhat=1.0, stats=stats), + ChangePoint(index=10, qhat=1.0, stats=stats), + ChangePoint(index=15, qhat=1.0, stats=stats), + ] + intervals = tester.get_intervals(sorted_cps) + assert len(intervals) == 4 + assert intervals[0] == slice(0, 5) + assert intervals[1] == slice(5, 10) + assert intervals[2] == slice(10, 15) + assert intervals[3] == slice(15, None) + + # Unsorted change points should raise AssertionError + unsorted_cps = [ + ChangePoint(index=10, qhat=1.0, stats=stats), + ChangePoint(index=5, qhat=1.0, stats=stats), + ChangePoint(index=15, qhat=1.0, stats=stats), + ] + with pytest.raises(AssertionError, match="Change points must be sorted by index"): + tester.get_intervals(unsorted_cps)
