From 40ce024051182f94c48541831fbb3223cdcd2ac1 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 4 May 2026 15:57:10 +0300 Subject: [PATCH 01/27] Rename the two different ChangePoint classes for clarity The recent work in #96 to replace the original external dependency on the so called "signal processing" repository with our own implementation, introduced new classess ChangePoint and CandidateChangePoint, in change_point_divisive/base.py but also left in place the original ChangePoint class in analysis.py. These come together in series.py, where the newer is renamed as _ChangePoint() and also acts as a parent to older class, thus aligning their signature as much as possible. It turns out having two similarly named classes can be a source of confusion and bugs. For example, in #141 vishnuchalla fixes a bug that is due to this and has essentially blocked the --orig-edivisive code path completely. This patch is an effort to make the existence of two separate classes very explicit, by renaming them to ChagePointHunter and ChangePointOtava based on their "lineage". A test case is added to exercise the --orig-edivisive code path. The test fails, as predicted by #141. The test is now cmmented out. The bug is due to a missing cp.metric property in one variation of the ChangePoint class. Note that this patch is intended more for discussion than to merge. --- otava/analysis.py | 15 ++- otava/bigquery.py | 4 +- otava/change_point_divisive/base.py | 14 +- otava/change_point_divisive/detector.py | 4 +- .../significance_test.py | 6 +- otava/postgres.py | 4 +- otava/series.py | 28 ++-- tests/change_point_divisive_test.py | 14 +- tests/cli_options_test.py | 123 ++++++++++++++++++ tests/postgres_e2e_test.py | 4 +- tests/report_test.py | 8 ++ tests/slack_notification_test.py | 1 + tests/tigerbeetle_test.py | 4 + 13 files changed, 184 insertions(+), 45 deletions(-) create mode 100644 tests/cli_options_test.py diff --git a/otava/analysis.py b/otava/analysis.py index 5ff89756..e57453be 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. +import copy from dataclasses import dataclass from typing import List, Optional, Sequence, SupportsFloat, Tuple @@ -24,7 +25,7 @@ from otava.change_point_divisive.base import ( BaseStats, CandidateChangePoint, - ChangePoint, + ChangePointOtava, GenericStats, SignificanceTester, ) @@ -97,11 +98,11 @@ def to_json(self): # Generic Change Point List -GenCPList = List[ChangePoint[GenericStats]] +GenCPList = List[ChangePointOtava[GenericStats]] # Permutation Change Point List -PermCPList = List[ChangePoint[PermutationStats]] +PermCPList = List[ChangePointOtava[PermutationStats]] # T-test Change Point List -TtestCPList = List[ChangePoint[TTestStats]] +TtestCPList = List[ChangePointOtava[TTestStats]] class TTestSignificanceTester(SignificanceTester): @@ -130,7 +131,7 @@ def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat]) def change_point( self, candidate: CandidateChangePoint, series: Sequence[SupportsFloat], intervals: List[slice] - ) -> ChangePoint[TTestStats]: + ) -> ChangePointOtava[TTestStats]: """ Computes properties of the change point if the Candidate Change Point based on the provided intervals. @@ -167,7 +168,7 @@ def change_point( left = series[left_interval] right = series[right_interval] stats = self.compare(left, right) - return ChangePoint.from_candidate(candidate, stats) + return ChangePointOtava.from_candidate(candidate, stats) def fill_missing(data: Sequence[SupportsFloat]): @@ -323,4 +324,4 @@ def compute_change_points( """ first_pass_pvalue = max_pvalue * 10 if max_pvalue < 0.05 else (max_pvalue * 2 if max_pvalue < 0.5 else max_pvalue) weak_change_points = split(series, window_len, first_pass_pvalue, new_points=new_data, old_cp=old_weak_cp) - return merge(weak_change_points, series, max_pvalue, min_magnitude), weak_change_points + return merge(copy.copy(weak_change_points), series, max_pvalue, min_magnitude), weak_change_points diff --git a/otava/bigquery.py b/otava/bigquery.py index 34f329d2..33816b96 100644 --- a/otava/bigquery.py +++ b/otava/bigquery.py @@ -22,7 +22,7 @@ from google.cloud import bigquery from google.oauth2 import service_account -from otava.analysis import ChangePoint +from otava.analysis import ChangePointOtava from otava.test_config import BigQueryTestConfig @@ -87,7 +87,7 @@ def insert_change_point( test: BigQueryTestConfig, metric_name: str, attributes: Dict, - change_point: ChangePoint, + change_point: ChangePointOtava, ): kwargs = {**attributes, **{test.time_column: datetime.utcfromtimestamp(change_point.time)}} update_stmt = test.update_stmt.format( diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index edde8e8d..e50d2d47 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -39,7 +39,7 @@ class BaseStats: @dataclass -class ChangePoint(CandidateChangePoint, Generic[GenericStats]): +class ChangePointOtava(CandidateChangePoint, Generic[GenericStats]): '''Change point class, defined by index and signigicance test statistic.''' stats: GenericStats @@ -48,7 +48,7 @@ def __eq__(self, other): return isinstance(other, self.__class__) and self.index == other.index @classmethod - def from_candidate(cls, candidate: CandidateChangePoint, stats: GenericStats) -> 'ChangePoint[GenericStats]': + def from_candidate(cls, candidate: CandidateChangePoint, stats: GenericStats) -> 'ChangePointOtava[GenericStats]': return cls( index=candidate.index, qhat=candidate.qhat, @@ -67,7 +67,7 @@ class SignificanceTester(Generic[GenericStats]): def __init__(self, max_pvalue: float): self.max_pvalue = max_pvalue - def get_intervals(self, change_points: List[ChangePoint[GenericStats]]) -> List[slice]: + def get_intervals(self, change_points: List[ChangePointOtava[GenericStats]]) -> List[slice]: '''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 @@ -82,12 +82,12 @@ def get_intervals(self, change_points: List[ChangePoint[GenericStats]]) -> List[ ] return [interval for interval in intervals if interval.start != interval.stop] - def is_significant(self, point: ChangePoint[GenericStats]) -> bool: - '''Compares ChangePoint to level of significance max_pvalue''' + def is_significant(self, point: ChangePointOtava[GenericStats]) -> bool: + '''Compares ChangePointOtava to level of significance max_pvalue''' return point.stats.pvalue <= self.max_pvalue - def change_point(self, candidate: CandidateChangePoint, series: NDArray, intervals: List[slice]) -> ChangePoint[GenericStats]: - '''Computes stats for a change point candidate and wraps it into ChangePoint class''' + def change_point(self, candidate: CandidateChangePoint, series: NDArray, intervals: List[slice]) -> ChangePointOtava[GenericStats]: + '''Computes stats for a change point candidate and wraps it into ChangePointOtava class''' ... diff --git a/otava/change_point_divisive/detector.py b/otava/change_point_divisive/detector.py index 89453720..e2026e98 100644 --- a/otava/change_point_divisive/detector.py +++ b/otava/change_point_divisive/detector.py @@ -21,7 +21,7 @@ from otava.change_point_divisive.base import ( Calculator, - ChangePoint, + ChangePointOtava, GenericStats, SignificanceTester, ) @@ -32,7 +32,7 @@ def __init__(self, significance_tester: SignificanceTester, calculator: Type[Cal self.tester = significance_tester self.calculator = calculator - def get_change_points(self, series: Sequence[SupportsFloat], start: Optional[int] = None, end: Optional[int] = None) -> List[ChangePoint[GenericStats]]: + def get_change_points(self, series: Sequence[SupportsFloat], start: Optional[int] = None, end: Optional[int] = None) -> List[ChangePointOtava[GenericStats]]: '''Finds change points in `series[start : end]`.''' if not isinstance(series, np.ndarray): series = np.array(series[start : end], dtype=np.float64) diff --git a/otava/change_point_divisive/significance_test.py b/otava/change_point_divisive/significance_test.py index 7e348c71..aa5a8f2c 100644 --- a/otava/change_point_divisive/significance_test.py +++ b/otava/change_point_divisive/significance_test.py @@ -25,7 +25,7 @@ BaseStats, Calculator, CandidateChangePoint, - ChangePoint, + ChangePointOtava, SignificanceTester, ) @@ -50,7 +50,7 @@ def __init__(self, max_pvalue: float, permutations: int, calculator: Type[Calcul self.seed = seed self.rng = np.random.default_rng(seed) - def change_point(self, candidate: CandidateChangePoint, series: NDArray, intervals: List[slice]) -> ChangePoint[PermutationStats]: + def change_point(self, candidate: CandidateChangePoint, series: NDArray, intervals: List[slice]) -> ChangePointOtava[PermutationStats]: '''Perform permutation test within candidate cluster''' # 1. Find permutated Qhats @@ -74,4 +74,4 @@ def change_point(self, candidate: CandidateChangePoint, series: NDArray, interva extreme_qhat_perm=extreme_qhat_perm, n_perm=self.permutations ) - return ChangePoint.from_candidate(candidate, stats) + return ChangePointOtava.from_candidate(candidate, stats) diff --git a/otava/postgres.py b/otava/postgres.py index 40ef53d3..5a30aa1e 100644 --- a/otava/postgres.py +++ b/otava/postgres.py @@ -21,7 +21,7 @@ import pg8000 -from otava.analysis import ChangePoint +from otava.analysis import ChangePointOtava from otava.test_config import PostgresTestConfig @@ -88,7 +88,7 @@ def insert_change_point( test: PostgresTestConfig, metric_name: str, attributes: Dict, - change_point: ChangePoint, + change_point: ChangePointOtava, ): cursor = self.__get_conn().cursor() kwargs = {**attributes, **{test.time_column: datetime.utcfromtimestamp(change_point.time)}} diff --git a/otava/series.py b/otava/series.py index d8cb3c4d..cd55159e 100644 --- a/otava/series.py +++ b/otava/series.py @@ -28,7 +28,7 @@ compute_change_points_orig, fill_missing, ) -from otava.change_point_divisive.base import ChangePoint as _ChangePoint +from otava.change_point_divisive.base import ChangePointOtava @dataclass @@ -73,7 +73,7 @@ def to_json(self): @dataclass -class ChangePoint(_ChangePoint[TTestStats]): +class ChangePointHunter(ChangePointOtava[TTestStats]): """A change-point for a single metric""" metric: str time: int @@ -141,7 +141,7 @@ class ChangePointGroup: prev_time: int attributes: Dict[str, str] prev_attributes: Dict[str, str] - changes: List[ChangePoint] + changes: List[ChangePointHunter] def to_json(self, rounded=False): return { @@ -216,11 +216,11 @@ class AnalyzedSeries: __series: Series options: AnalysisOptions - change_points: Dict[str, List[ChangePoint]] + change_points: Dict[str, List[ChangePointHunter]] change_points_by_time: List[ChangePointGroup] change_points_timestamp: Any - def __init__(self, series: Series, options: AnalysisOptions, change_points: Dict[str, ChangePoint] = None): + def __init__(self, series: Series, options: AnalysisOptions, change_points: Dict[str, ChangePointHunter] = None): self.__series = series self.options = options self.change_points_timestamp = datetime.now(tz=timezone.utc) @@ -236,7 +236,7 @@ def __init__(self, series: Series, options: AnalysisOptions, change_points: Dict @staticmethod def __compute_change_points( series: Series, options: AnalysisOptions - ) -> Dict[str, List[ChangePoint]]: + ) -> Dict[str, List[ChangePointHunter]]: result = {} weak_change_points = {} for metric in series.data.keys(): @@ -268,13 +268,13 @@ def __compute_change_points( ) for c in weak_cps: weak_change_points[metric].append( - ChangePoint( + ChangePointHunter( index=c.index, qhat=0.0, time=series.time[c.index], metric=metric, stats=c.stats ) ) for c in change_points: result[metric].append( - ChangePoint( + ChangePointHunter( index=c.index, qhat=0.0, time=series.time[c.index], metric=metric, stats=c.stats ) ) @@ -284,9 +284,9 @@ def __compute_change_points( @staticmethod def __group_change_points_by_time( - series: Series, change_points: Dict[str, List[ChangePoint]] + series: Series, change_points: Dict[str, List[ChangePointHunter]] ) -> List[ChangePointGroup]: - changes: List[ChangePoint] = [] + changes: List[ChangePointHunter] = [] for metric in change_points.keys(): changes += change_points[metric] @@ -391,14 +391,14 @@ def append(self, time, new_data, attributes): result[metric] = [] for c in change_points: result[metric].append( - ChangePoint( + ChangePointHunter( index=c.index, qhat=0.0, time=self.__series.time[c.index], metric=metric, stats=c.stats ) ) weak_change_points[metric] = [] for c in weak_cps: weak_change_points[metric].append( - ChangePoint( + ChangePointHunter( index=c.index, qhat=0.0, time=self.__series.time[c.index], metric=metric, stats=c.stats ) ) @@ -503,7 +503,7 @@ def from_json(cls, analyzed_json): pvalue=cp["pvalue"], ) new_list.append( - ChangePoint( + ChangePointHunter( index=cp["index"], time=cp["time"], metric=cp["metric"], stats=stat ) ) @@ -521,7 +521,7 @@ def from_json(cls, analyzed_json): pvalue=cp["pvalue"], ) new_list.append( - ChangePoint( + ChangePointHunter( index=cp["index"], time=cp["time"], metric=cp["metric"], stats=stat ) ) diff --git a/tests/change_point_divisive_test.py b/tests/change_point_divisive_test.py index 45d44318..950d982a 100644 --- a/tests/change_point_divisive_test.py +++ b/tests/change_point_divisive_test.py @@ -19,7 +19,7 @@ import pytest from otava.analysis import TTestSignificanceTester, TTestStats -from otava.change_point_divisive.base import ChangePoint +from otava.change_point_divisive.base import ChangePointOtava 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 @@ -124,9 +124,9 @@ def test_get_intervals_requires_sorted_change_points(): # 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), + ChangePointOtava(index=5, qhat=1.0, stats=stats), + ChangePointOtava(index=10, qhat=1.0, stats=stats), + ChangePointOtava(index=15, qhat=1.0, stats=stats), ] intervals = tester.get_intervals(sorted_cps) assert len(intervals) == 4 @@ -137,9 +137,9 @@ def test_get_intervals_requires_sorted_change_points(): # 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), + ChangePointOtava(index=10, qhat=1.0, stats=stats), + ChangePointOtava(index=5, qhat=1.0, stats=stats), + ChangePointOtava(index=15, qhat=1.0, stats=stats), ] with pytest.raises(AssertionError, match="Change points must be sorted by index"): tester.get_intervals(unsorted_cps) diff --git a/tests/cli_options_test.py b/tests/cli_options_test.py new file mode 100644 index 00000000..d578f14b --- /dev/null +++ b/tests/cli_options_test.py @@ -0,0 +1,123 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import csv +import tempfile +import textwrap +import unittest +from datetime import datetime, timedelta +from pathlib import Path +from unittest.mock import patch + +import otava.series +from otava.main import script_main + + +class CliOptionsTest(unittest.TestCase): + + # Test --deterministic-edivisive in various ways + def test_no_cli_option(self): + with patch('otava.series.compute_change_points') as mock_split: + script_main(args=[]) + mock_split.assert_not_called() + + def test_default_cli_option(self): + with patch('otava.series.compute_change_points') as mock_split: + mock_split.return_value = ([], []) + with tempfile.TemporaryDirectory() as td: + td_path = Path(td) + csv_path, timestamps, config_path, test_name = _create_files_in_temp_dir(td_path) + # _uv_run(td_path, test_name) + config_path_str = "" + str(config_path) + script_main(args=["analyze", "--config", config_path_str, test_name]) + + assert otava.series.compute_change_points.call_count == 2 + + # Failing due to lack of cp.metric see pull#141 + # def test_orig_cli_option(self): + # with patch('otava.series.compute_change_points') as mock_orig: + # mock_orig.return_value = ([], None) + # with tempfile.TemporaryDirectory() as td: + # td_path = Path(td) + # csv_path, timestamps, config_path, test_name = _create_files_in_temp_dir(td_path) + # # _uv_run(td_path, test_name) + # config_path_str = "" + str(config_path) + # script_main(args=["analyze", "--config", config_path_str, "--orig-edivisive", "true", test_name]) + # + # assert otava.series.compute_change_points_orig.call_count == 2 + + +def _create_files_in_temp_dir(td_path: Path): + data_dir = td_path / "data" + data_dir.mkdir(parents=True, exist_ok=True) + + csv_path, timestamps = _create_csv_data_file_for_test(td_path) + config_path, test_name = _create_csv_config_file_for_test(td_path) + + return csv_path, timestamps, config_path, test_name + + +def _create_csv_data_file_for_test(td_path: Path): + # create data directory and write CSV + data_dir = td_path / "data" + csv_path = data_dir / "local_sample.csv" + + # Generate some CSV content + now = datetime.now() + n = 10 + timestamps = [now - timedelta(days=i) for i in range(n)] + metrics1 = [154023, 138455, 143112, 149190, 132098, 151344, 155145, 148889, 149466, 148209] + metrics2 = [10.43, 10.23, 10.29, 10.91, 10.34, 10.69, 9.23, 9.11, 9.13, 9.03] + data_points = [] + for i in range(n): + data_points.append( + ( + timestamps[i].strftime("%Y.%m.%d %H:%M:%S %z"), # time + "aaa" + str(i), # commit + metrics1[i], + metrics2[i], + ) + ) + + with open(csv_path, "w", newline="") as f: + writer = csv.writer(f) + writer.writerow(["time", "commit", "metric1", "metric2"]) + writer.writerows(data_points) + + return csv_path, timestamps + + +def _create_csv_config_file_for_test(td_path: Path): + data_dir = td_path / "data" + csv_path = str(data_dir / "local_sample.csv") + + config_content = textwrap.dedent( + """\ + tests: + sample_for_test: + type: csv + file: """ + csv_path + """ + time_column: time + attributes: [commit] + metrics: [metric1, metric2] + csv_options: + delimiter: "," + quotechar: "'" + """ + ) + config_path = td_path / "otava.yaml" + config_path.write_text(config_content, encoding="utf-8") + return config_path, "sample_for_test" diff --git a/tests/postgres_e2e_test.py b/tests/postgres_e2e_test.py index 5da97688..58bb03ec 100644 --- a/tests/postgres_e2e_test.py +++ b/tests/postgres_e2e_test.py @@ -80,7 +80,9 @@ def test_analyze(): 2025-04-27 10:03:02 +0000 aggregate-0af4ccbc 0af4ccbc 1 56950 2052 13532 """ ) - assert _remove_trailing_whitespaces(proc.stdout) == expected_output.rstrip("\n") + out = _remove_trailing_whitespaces(proc.stdout) + print(out) + assert out == expected_output.rstrip("\n") # Verify the DB was updated with the detected change. # Query the updated change metric at the detected change point. diff --git a/tests/report_test.py b/tests/report_test.py index 3692a799..a220eb55 100644 --- a/tests/report_test.py +++ b/tests/report_test.py @@ -40,6 +40,13 @@ def series(): @pytest.fixture(scope="module") def change_points(series): + # o = AnalysisOptions() + # print("default AnalysisOptions are...") + # print(o) + # o.max_pvalue = 0.001 + # print("setting them to") + # print(o) + # return series.analyze(options=o).change_points_by_time return series.analyze().change_points_by_time @@ -51,6 +58,7 @@ def report(series, change_points): def test_report(series, change_points): report = Report(series, change_points) output = report.produce_report("test", ReportType.LOG) + assert "series1" in output assert "series2" in output assert "1.02" in output diff --git a/tests/slack_notification_test.py b/tests/slack_notification_test.py index 7bc47054..0f0b81eb 100644 --- a/tests/slack_notification_test.py +++ b/tests/slack_notification_test.py @@ -99,6 +99,7 @@ def test_blocks_dispatch(): since=since_time, ) dispatches = mock_client.dispatches + assert list(dispatches.keys()) == NOTIFICATION_CHANNELS, "Wrong channels were notified" for channel in NOTIFICATION_CHANNELS: assert len(dispatches[channel]) == 1, "Unexpected number of Slack messages created" diff --git a/tests/tigerbeetle_test.py b/tests/tigerbeetle_test.py index 94a90195..77ea5241 100644 --- a/tests/tigerbeetle_test.py +++ b/tests/tigerbeetle_test.py @@ -19,6 +19,10 @@ from otava.analysis import compute_change_points +def tigerbeetle_demo_data(): + return _get_series() + + def _get_series(): """ This is the Tigerbeetle dataset used for demo purposes at Nyrkiö. From bd935311c4c4ad715239bbde04529feb52377995 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Sat, 30 May 2026 00:32:12 +0300 Subject: [PATCH 02/27] Unify the two ChangePoint classes and add container classes * Unify the ChangePoint_ class in hunter code and the new ChangePoint introduced by the new edivisive implementation Then it got out of hand a bit ... * Separate index and timestamp into different domains. cp.index is used in the context of a single metric and its history of results. Time and commit otoh are on the ChangePointGroup level (essentially a "row"). Note that different metrics can now have different cp.index for the same cpg.time or cpg.attributes['commit'], if they have a different history. * Introduce a ChangePoints class which is just a list of ChangePointGroups but actually comes with 2 different implementations. The last one is supposed to become the class you are left holding once all the change points are computed. Until now we had lots of nice classes for each step of computation, but in the end you were left holding a dict[str, ChangePointGroup]. The new class now encapsulates that dict, --- otava/analysis.py | 23 +- otava/bigquery.py | 4 +- otava/change_point_divisive/base.py | 494 +++++++++++++++++- otava/change_point_divisive/detector.py | 4 +- .../significance_test.py | 6 +- otava/main.py | 5 +- otava/postgres.py | 7 +- otava/report.py | 15 +- otava/series.py | 245 +++------ otava/slack.py | 11 +- tests/change_point_divisive_test.py | 14 +- tests/report_test.py | 9 +- tests/series_test.py | 119 ++++- 13 files changed, 704 insertions(+), 252 deletions(-) diff --git a/otava/analysis.py b/otava/analysis.py index e57453be..84c69c2c 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -25,7 +25,7 @@ from otava.change_point_divisive.base import ( BaseStats, CandidateChangePoint, - ChangePointOtava, + ChangePoint, GenericStats, SignificanceTester, ) @@ -98,11 +98,11 @@ def to_json(self): # Generic Change Point List -GenCPList = List[ChangePointOtava[GenericStats]] +GenCPList = List[ChangePoint[GenericStats]] # Permutation Change Point List -PermCPList = List[ChangePointOtava[PermutationStats]] +PermCPList = List[ChangePoint[PermutationStats]] # T-test Change Point List -TtestCPList = List[ChangePointOtava[TTestStats]] +TtestCPList = List[ChangePoint[TTestStats]] class TTestSignificanceTester(SignificanceTester): @@ -112,6 +112,7 @@ class TTestSignificanceTester(SignificanceTester): This test is good if the data between the change points have normal distribution. It works well even with tiny numbers of points (<10). """ + def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat]) -> TTestStats: if len(left) == 0 or len(right) == 0: raise ValueError @@ -130,8 +131,11 @@ def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat]) return TTestStats(mean_1=mean_l, mean_2=mean_r, std_1=std_l, std_2=std_r, pvalue=p) def change_point( - self, candidate: CandidateChangePoint, series: Sequence[SupportsFloat], intervals: List[slice] - ) -> ChangePointOtava[TTestStats]: + self, + candidate: CandidateChangePoint, + series: Sequence[SupportsFloat], + intervals: List[slice], + ) -> ChangePoint[TTestStats]: """ Computes properties of the change point if the Candidate Change Point based on the provided intervals. @@ -164,11 +168,13 @@ def change_point( right_interval = slice(candidate.index, interval.stop) break else: - raise ValueError(f"Candidate Change Point at index={candidate.index} doesn't correspond to any interval in {intervals}.") + raise ValueError( + f"Candidate Change Point at index={candidate.index} doesn't correspond to any interval in {intervals}." + ) left = series[left_interval] right = series[right_interval] stats = self.compare(left, right) - return ChangePointOtava.from_candidate(candidate, stats) + return ChangePoint.from_candidate(candidate, stats) def fill_missing(data: Sequence[SupportsFloat]): @@ -202,7 +208,6 @@ def merge( """ tester = TTestSignificanceTester(max_pvalue) while change_points: - # Select the change point with weakest unacceptable P-value # If all points have acceptable P-values, select the change-point with # the least relative change: diff --git a/otava/bigquery.py b/otava/bigquery.py index 33816b96..34f329d2 100644 --- a/otava/bigquery.py +++ b/otava/bigquery.py @@ -22,7 +22,7 @@ from google.cloud import bigquery from google.oauth2 import service_account -from otava.analysis import ChangePointOtava +from otava.analysis import ChangePoint from otava.test_config import BigQueryTestConfig @@ -87,7 +87,7 @@ def insert_change_point( test: BigQueryTestConfig, metric_name: str, attributes: Dict, - change_point: ChangePointOtava, + change_point: ChangePoint, ): kwargs = {**attributes, **{test.time_column: datetime.utcfromtimestamp(change_point.time)}} update_stmt = test.update_stmt.format( diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index e50d2d47..275a61fa 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -14,23 +14,58 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +""" +Hierarchy of ChangePoint classes: + CandidateChangePoint <--> ChangePoint --> ChangePointSerializer + .index .index .to_json() + , .stats .get_this_or_that() + ^ + / `------BaseStats + ^ .pvalue + / ` + `GenericStats + / TTestStats + PermutationStats + / + ChangePointGroup + .time + .attributes.commit + .changes[metric, ChangePoint] + # Essentially a row: One or more ChangePoint at the same commit/time + + / + ChangePoints + # Typically all change points for a given test / run / etc + ^ + | + ^ + ChangePointsByTime `ChangePointsByMetric + .change_points: list(ChangePointGroup) .change_points: dict[metric, list(ChangePointGroup)] + .pivot() < - - > .pivot() +""" + +from collections import OrderedDict from dataclasses import dataclass, fields -from typing import Generic, List, Optional, TypeVar +from datetime import UTC, datetime +from typing import Dict, Generic, List, Optional, TypeVar from numpy.typing import NDArray @dataclass class CandidateChangePoint: - '''Candidate for a change point. The point that maximizes Q-hat function on [start:end+1] slice''' + """Candidate for a change point. The point that maximizes Q-hat function on [start:end+1] slice""" + index: int qhat: float @dataclass class BaseStats: - '''Abstract statistics class for change point. Implementation depends on the statistical test.''' + """Abstract statistics class for change point. Implementation depends on the statistical test.""" + + # The pvalue for this change point. Exact value depends on the algorithm that was used. pvalue: float @@ -39,16 +74,36 @@ class BaseStats: @dataclass -class ChangePointOtava(CandidateChangePoint, Generic[GenericStats]): - '''Change point class, defined by index and signigicance test statistic.''' +class ChangePoint(CandidateChangePoint, Generic[GenericStats]): + """ + ChangePoint class. + + Defined by index and signigicance test statistic. + This class is the basic change point that is used during computation + and returned as a result. This class does not however carry additional + attributes like metric, time, or commit sha. Those are in ChangePointGroup + and ChangePoints. + Note that while in theory the index, commit sha, an the time(stamp) should + all be the same, in practice they aren't always. For example if at some point + during a tests lifetime, more metrics are added to the output, then different + metrics will have different histories and therefore their indexes start from + different locations. + To use time(stamp), metric name or timestamp, to access change points, please + use the ChangePointGroup and ChangePoints classes. + """ + stats: GenericStats + # Which metric this change point belongs to. (This is redundant and for convenience.) + metric: Optional[str] = None def __eq__(self, other): - '''Helpful to identify new Change Points during divisive algorithm''' + """Helpful to identify new Change Points during divisive algorithm""" return isinstance(other, self.__class__) and self.index == other.index @classmethod - def from_candidate(cls, candidate: CandidateChangePoint, stats: GenericStats) -> 'ChangePointOtava[GenericStats]': + def from_candidate( + cls, candidate: CandidateChangePoint, stats: GenericStats + ) -> "ChangePoint[GenericStats]": return cls( index=candidate.index, qhat=candidate.qhat, @@ -56,19 +111,417 @@ def from_candidate(cls, candidate: CandidateChangePoint, stats: GenericStats) -> ) def to_candidate(self) -> CandidateChangePoint: - '''Downgrades Change Point to a Candidate Change Point. Used to recompute stats for Weak Change Points.''' + """Downgrades Change Point to a Candidate Change Point. Used to recompute stats for Weak Change Points.""" data = {f.name: getattr(self, f.name) for f in fields(CandidateChangePoint)} return CandidateChangePoint(**data) + def to_json(self, rounded=True): + cps = ChangePointSerializer(self) + return cps.to_json(rounded) + + +class ChangePointSerializer(ChangePoint): + """ + Utility class with getters and json serialization for a ChangePoint. + + TODO: Maintaining this is tedious. We should replace it with pydantic or some + other standard solution that provides json serialization. + """ + + def __init__(self, cp: ChangePoint[GenericStats]): + self.stats = cp.stats + self.index = cp.index + self.metric = cp.metric + + def forward_change_percent(self) -> float: + return self.stats.forward_rel_change() * 100.0 + + def backward_change_percent(self) -> float: + return self.stats.backward_rel_change() * 100.0 + + def magnitude(self): + return self.stats.change_magnitude() + + def mean_before(self): + return self.stats.mean_1 + + def mean_after(self): + return self.stats.mean_2 + + def stddev_before(self): + return self.stats.std_1 + + def stddev_after(self): + return self.stats.std_2 + + def pvalue(self): + return self.stats.pvalue + + def to_json(self, rounded=True): + if rounded: + return { + "metric": self.metric, + "index": int(self.index), + "forward_change_percent": f"{self.forward_change_percent():.0f}", + "magnitude": f"{self.magnitude():-0f}", + "mean_before": f"{self.mean_before():-0f}", + "stddev_before": f"{self.stddev_before():-0f}", + "mean_after": f"{self.mean_after():-0f}", + "stddev_after": f"{self.stddev_after():-0f}", + "pvalue": f"{self.pvalue():-0f}", + } + + else: + return { + "metric": self.metric, + "index": int(self.index), + "forward_change_percent": self.forward_change_percent(), + "magnitude": self.magnitude(), + "mean_before": self.mean_before(), + "stddev_before": self.stddev_before(), + "mean_after": self.mean_after(), + "stddev_after": self.stddev_after(), + "pvalue": self.pvalue(), + } + + +@dataclass +class ChangePointGroup: + """A group of change points on multiple metrics, at the same time""" + + time: float + attributes: Dict[str, str] + # ChangePointGroup.changes.keys() stores the set of metrics that were used at this ChangePointGroup.time. + changes: Dict[str, ChangePoint] + + def to_json(self, rounded=False): + changes = [] + for metric, cp in self.changes.items(): + changes.append(cp.to_json(rounded=rounded)) + + return { + "time": self.time, + "attributes": self.attributes, + "changes": changes, + } + + def __getitem__(self, metric): + return self.changes[metric] + + def metrics(self): + return self.changes.keys() + + def commit(self, idx: int): + return self.attribute_at(idx).get("commit") + + def datetime(self): + return datetime.fromtimestamp(self.time, UTC) + + def select_metrics(self, m: list[str] | str): + if not isinstance(m, list): + m = [m] + filtered = ChangePointGroup(time=self.time, attributes=self.attributes, changes={}) + for metric, cp in self.changes.items(): + if metric in m: + filtered.changes[metric] = cp + return filtered + + def set(self, metric: str, cp: ChangePoint): + self.changes[metric] = cp + + def __iter__(self): + return iter([v for v in list(self.changes.values())]) + + +class ChangePoints: + """ + A list of ChangePointGroup objects. + + Typical usage of this would be to hold all the change points over a history of a single test, + the test producing one or more metrics. Note that this is a sparse structure: It is NOT + guaranteed that each row (each GhangePointGroup) has each metric. Similarly it is not guaranteed + that a given metric will hold the full sequence. + + Companion class ChangePointsByMetric is expected to provide functionally equivalent interface, but + storing each series separately by metric, which is used in parts of the code base, in particular, what + Series.analyze() returns. + Subclass ChangePointsByTime is this same class, but can be used if you explicitly want to mark the ordering. + """ + + def __init__(self, cps=None): + if isinstance(cps, dict) and not isinstance(cps, OrderedDict): + raise TypeError( + "ChangePointsByTime doesn't accept a dict() as constructor input. Did you want ChangePointsByMetric()?" + ) + if isinstance(cps, OrderedDict): + for k, v in cps.items(): + if not isinstance(k, float): + raise TypeError( + "ChangePointsByTime with OrderedDict() as constructor input requires the keys to be float (timestamps)?" + ) + if not isinstance(v, ChangePointGroup): + raise TypeError( + "ChangePointsByTime input must be an OrderedDict() of ChangePointGroup objects as values." + ) + self.change_points = sorted(cps, key=lambda cpg: cpg.time) + return + if cps is None: + self.change_points = [] + return + + if isinstance(cps, ChangePointsByTime): + self.change_points = sorted(cps.change_points, key=lambda cpg: cpg.time) + return + if isinstance(cps, ChangePointsByMetric): + self.change_points = cps.pivot().change_points + return + + if not isinstance(cps, list): + cps = [cps] + for obj in sorted(cps, key=lambda cpg: cpg.time): + if not isinstance(obj, ChangePointGroup): + t = type(obj) + raise TypeError( + f"ChangePoints() takes as argument one or more ChangePointGroup objects. Got {t}." + ) + self.change_points = cps + + def append(self, cpg: ChangePointGroup): + if not isinstance(cpg, ChangePointGroup): + raise TypeError("ChangePoints.append() takes as argument one ChangePointGroup.") + + if (not self.change_points) or cpg.time > self.change_points[-1].time: + self.change_points.append(cpg) + elif self.change_points and cpg.time == self.change_points[-1].time: + for metric, cp in cpg.changes.items(): + if metric in self.change_points[-1].changes: + raise KeyError("Duplicate keys. Shouldn't happen.") + self.change_points[-1].changes[metric] = cp + else: + # TODO: logging + # print(self.change_points) + # print(cpg) + raise ValueError( + "ChangePoints.append() can only be used such that time is monotonically increasing" + ) + + def extend(self, cps): + errmsg = "ChangePoints.extend() takes as argument a list of ChangePointGroup objects." + if not isinstance(cps, list): + raise TypeError(errmsg) + for obj in cps: + if not isinstance(obj, ChangePointGroup): + raise TypeError(errmsg) + if (not self.change_points) or obj.time > self.change_points[-1].time: + self.change_points.append(obj) + else: + raise ValueError( + "ChangePoints.extend() can only be used such that time is monotonically increasing" + ) + + def items(self): + return self.pivot().items() + + def __iter__(self): + return iter(self.change_points) + + def __len__(self): + return len(self.change_points) + + def __getitem__(self, n): + return self.change_points[n] + + def metrics(self) -> set: + all_metrics = set() + for row in self.change_points: + all_metrics.add(row.metrics()) + return all_metrics + + def select_metrics(self, m: list[str] | str): + """ + Get a new ChangePoints object holding only the given metric(s). + + If you think of a ChangePoints object as timestamps being rows, and + the metrics being columns, then this returns a single column. + + Note: The internal data structure doesn't do anything to make this + request efficient. This will loop over all ChangePointGroups. + Use ChangePointsByMetric if you need this to be fast. + """ + filtered = ChangePoints() + for cpg in self.change_points: + filtered.append(cpg.select_metrics(m)) + return filtered + + def get_change_points_for_metric(self, m: str): + single_metric = self.select_metrics(m) + return [list(cpg.changes.values())[0] for cpg in single_metric.change_points] + + def at_timestamp(self, t: float): + for cpg in self.change_points: + if cpg.time == t: + return cpg + if abs(cpg.time - t) < 0.0001: + return cpg + raise LookupError(t) + + def at_commit(self, sha: str): + for row in self: + if row.attributes['commit'] == sha: + return row + raise LookupError(sha) + + def pivot(self): + """ + Return the same object as ChangePointsByMetric. + """ + by_metric = ChangePointsByMetric() + + for row in sorted(self.change_points, key=lambda cpg: cpg.time): + assert isinstance(row, ChangePointGroup) + # append() does the necessary shuffling into separate columns + by_metric.append(row) + + +class ChangePointsByTime(ChangePoints): + pass + + +class ChangePointsByMetric(ChangePoints): + """ + Provides same interface as ChangePoints, but internally stores with metric first. + """ + + def __init__(self, cps=None): + if isinstance(cps, ChangePointsByMetric): + self.change_points = cps.change_points + if isinstance(cps, ChangePointsByTime): + self.change_points = cps.pivot().change_points + + if cps is None: + self.change_points = OrderedDict() + return + if isinstance(cps, list): + cpm = [] + for obj in sorted(cps, key=lambda cpg: cpg.time): + if not isinstance(obj, ChangePointGroup): + t = type(obj) + raise TypeError( + f"ChangePointsByMetric() takes as input a list of ChangePointGroup objects. Got {t}." + ) + cpm.append(obj) + + self.change_points = cpm + if isinstance(cps, dict): + # We actually don't need the ordering in this case, but we want the type to match the other class + self.change_points = OrderedDict() + for metric, cpglist in cps.items(): + for cpg in cpglist: + if not isinstance(cpg, ChangePointGroup): + raise TypeError( + "ChangePointsByMetric takes as constructor argument a dict of ChangePointGroup objects: dict[str, list[ChangePointGroup]]" + ) + self.change_points[metric] = sorted(cpglist, key=lambda cpg: cpg.time) + + def pivot(self): + # Now we pivot (metric,time) to (time,metric) so that we return ChangePoints() objects + intermediate = [] + for metric, points in self.change_points.items(): + for cpg in sorted(points, key=lambda cpg: cpg.time): + assert isinstance(cpg, ChangePointGroup) + intermediate.append(cpg) + cp_by_time = ChangePointsByTime() + for cpg in sorted(intermediate, key=lambda cpg: cpg.time): + cp_by_time.append(cpg) + return cp_by_time + + def append(self, cpg: ChangePointGroup): + if not isinstance(cpg, ChangePointGroup): + raise TypeError("ChangePoints.append() takes as argument one ChangePointGroup.") + for metric in cpg.metrics(): + self.change_points[metric].append(cpg.select_metrics(metric)) + + def extend(self, cps): + errmsg = "ChangePoints.extend() takes as argument a list of ChangePointGroup objects." + if not isinstance(cps, list): + raise TypeError(errmsg) + for obj in cps: + if not isinstance(obj, ChangePointGroup): + raise TypeError(errmsg) + self.append(obj) + + def items(self): + return self.change_points.items() + + def __iter__(self): + return self.pivot().__iter__() + + def __len__(self): + return max([len(cpg) for metric, cpg in self.change_points.items()]) + + def __getitem__(self, n): + if not isinstance(n, int): + raise KeyError("n must be integer index") + cpgrow = [cpg[n] for metric, cpg in self.change_points.items() if len(cpg) > n] + if not cpgrow: + KeyError(f"Nice try {n}. This ChangePoints object only has {len(self)} items.") + cpg = None + for c in cpgrow: + if cpg is None: + # First element + cpg = c + continue + for metric in cpg.metrics(): + cpg[metric] = c + + return cpg + + def metrics(self): + return set(self.change_points.keys()) + + def select_metrics(self, m: list[str] | str): + """ + Get a new ChangePoints object holding only the given metric(s). + """ + if not isinstance(m, list): + if not isinstance(m, str): + TypeError("ChangePoints.select_metrics() takes as argument a str or a list of str.") + m = [m] + + filtered = ChangePointsByMetric() + for metric in m: + filtered.change_points[metric] = self.change_points[metric] + return filtered + + def get_change_points_for_metric(self, m: str): + single_metric = self.select_metrics(m) + metric_change_points = [] + for metric, cpg in single_metric.change_points.items(): + for c in cpg: + metric_change_points.append(c.changes[metric]) + return metric_change_points + + def at_timestamp(self, t: float): + """ + This is slow, please consider using ChangePointsByTime instead. + """ + return self.pivot().at_timestamp(t) + + def at_commit(self, sha: str): + """ + This is slow, please consider using ChangePointsByTime instead. + """ + return self.pivot().at_commit(sha) + class SignificanceTester(Generic[GenericStats]): - '''Abstract class for significance tester''' + """Abstract class for significance tester""" def __init__(self, max_pvalue: float): self.max_pvalue = max_pvalue - def get_intervals(self, change_points: List[ChangePointOtava[GenericStats]]) -> List[slice]: - '''Returns list of slices of the series. Change points must be sorted by index.''' + def get_intervals(self, change_points: List[ChangePoint[GenericStats]]) -> List[slice]: + """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) @@ -82,22 +535,25 @@ def get_intervals(self, change_points: List[ChangePointOtava[GenericStats]]) -> ] return [interval for interval in intervals if interval.start != interval.stop] - def is_significant(self, point: ChangePointOtava[GenericStats]) -> bool: - '''Compares ChangePointOtava to level of significance max_pvalue''' + def is_significant(self, point: ChangePoint[GenericStats]) -> bool: + """Compares ChangePoint to level of significance max_pvalue""" return point.stats.pvalue <= self.max_pvalue - def change_point(self, candidate: CandidateChangePoint, series: NDArray, intervals: List[slice]) -> ChangePointOtava[GenericStats]: - '''Computes stats for a change point candidate and wraps it into ChangePointOtava class''' + def change_point( + self, candidate: CandidateChangePoint, series: NDArray, intervals: List[slice] + ) -> ChangePoint[GenericStats]: + """Computes stats for a change point candidate and wraps it into ChangePoint class""" ... class Calculator: - '''Abstract class for calculator. Calculator provides an interface to get best change point candidate''' + """Abstract class for calculator. Calculator provides an interface to get best change point candidate""" + def __init__(self, series: NDArray): self.series = series def get_next_candidate(self, intervals: List[slice]) -> Optional[CandidateChangePoint]: - '''Returns list of existing change points to find next best change point candidate.''' + """Returns list of existing change points to find next best change point candidate.""" candidates = [ self.get_candidate_change_point(interval=interval) for interval in intervals @@ -109,6 +565,6 @@ def get_next_candidate(self, intervals: List[slice]) -> Optional[CandidateChange return candidate def get_candidate_change_point(self, interval: slice) -> CandidateChangePoint: - '''Given start and end indexes return best candidate for a change point. - Note that start and end are indexes of the first and last element, i.e. a slice [start:end+1].''' + """Given start and end indexes return best candidate for a change point. + Note that start and end are indexes of the first and last element, i.e. a slice [start:end+1].""" ... diff --git a/otava/change_point_divisive/detector.py b/otava/change_point_divisive/detector.py index e2026e98..89453720 100644 --- a/otava/change_point_divisive/detector.py +++ b/otava/change_point_divisive/detector.py @@ -21,7 +21,7 @@ from otava.change_point_divisive.base import ( Calculator, - ChangePointOtava, + ChangePoint, GenericStats, SignificanceTester, ) @@ -32,7 +32,7 @@ def __init__(self, significance_tester: SignificanceTester, calculator: Type[Cal self.tester = significance_tester self.calculator = calculator - def get_change_points(self, series: Sequence[SupportsFloat], start: Optional[int] = None, end: Optional[int] = None) -> List[ChangePointOtava[GenericStats]]: + def get_change_points(self, series: Sequence[SupportsFloat], start: Optional[int] = None, end: Optional[int] = None) -> List[ChangePoint[GenericStats]]: '''Finds change points in `series[start : end]`.''' if not isinstance(series, np.ndarray): series = np.array(series[start : end], dtype=np.float64) diff --git a/otava/change_point_divisive/significance_test.py b/otava/change_point_divisive/significance_test.py index aa5a8f2c..7e348c71 100644 --- a/otava/change_point_divisive/significance_test.py +++ b/otava/change_point_divisive/significance_test.py @@ -25,7 +25,7 @@ BaseStats, Calculator, CandidateChangePoint, - ChangePointOtava, + ChangePoint, SignificanceTester, ) @@ -50,7 +50,7 @@ def __init__(self, max_pvalue: float, permutations: int, calculator: Type[Calcul self.seed = seed self.rng = np.random.default_rng(seed) - def change_point(self, candidate: CandidateChangePoint, series: NDArray, intervals: List[slice]) -> ChangePointOtava[PermutationStats]: + def change_point(self, candidate: CandidateChangePoint, series: NDArray, intervals: List[slice]) -> ChangePoint[PermutationStats]: '''Perform permutation test within candidate cluster''' # 1. Find permutated Qhats @@ -74,4 +74,4 @@ def change_point(self, candidate: CandidateChangePoint, series: NDArray, interva extreme_qhat_perm=extreme_qhat_perm, n_perm=self.permutations ) - return ChangePointOtava.from_candidate(candidate, stats) + return ChangePoint.from_candidate(candidate, stats) diff --git a/otava/main.py b/otava/main.py index 36671b67..87fb4829 100644 --- a/otava/main.py +++ b/otava/main.py @@ -243,9 +243,10 @@ def __get_bigquery(self) -> BigQuery: def update_postgres(self, test: PostgresTestConfig, series: AnalyzedSeries): postgres = self.__get_postgres() for metric_name, change_points in series.change_points.items(): - for cp in change_points: + for cpg in change_points: + cp = cpg[metric_name] attributes = series.attributes_at(cp.index) - postgres.insert_change_point(test, metric_name, attributes, cp) + postgres.insert_change_point(test, metric_name, attributes, cpg) def update_bigquery(self, test: BigQueryTestConfig, series: AnalyzedSeries): bigquery = self.__get_bigquery() diff --git a/otava/postgres.py b/otava/postgres.py index 5a30aa1e..cbf22af6 100644 --- a/otava/postgres.py +++ b/otava/postgres.py @@ -21,7 +21,7 @@ import pg8000 -from otava.analysis import ChangePointOtava +from otava.change_point_divisive.base import ChangePointGroup, ChangePointSerializer from otava.test_config import PostgresTestConfig @@ -88,10 +88,11 @@ def insert_change_point( test: PostgresTestConfig, metric_name: str, attributes: Dict, - change_point: ChangePointOtava, + change_point_group: ChangePointGroup, ): cursor = self.__get_conn().cursor() - kwargs = {**attributes, **{test.time_column: datetime.utcfromtimestamp(change_point.time)}} + change_point = ChangePointSerializer(change_point_group[metric_name]) + kwargs = {**attributes, **{test.time_column: datetime.utcfromtimestamp(change_point_group.time)}} update_stmt = test.update_stmt.format(metric=metric_name, **kwargs) cursor.execute( update_stmt, diff --git a/otava/report.py b/otava/report.py index d9cf3c43..1244af07 100644 --- a/otava/report.py +++ b/otava/report.py @@ -21,7 +21,8 @@ from tabulate import tabulate -from otava.series import ChangePointGroup, Series +from otava.change_point_divisive.base import ChangePoints, ChangePointSerializer +from otava.series import Series from otava.util import format_timestamp, insert_multiple, remove_common_prefix @@ -37,9 +38,9 @@ def __str__(self): class Report: __series: Series - __change_points: List[ChangePointGroup] + __change_points: ChangePoints - def __init__(self, series: Series, change_points: List[ChangePointGroup]): + def __init__(self, series: Series, change_points: ChangePoints): self.__series = series self.__change_points = change_points @@ -74,19 +75,19 @@ def __format_log_annotated(self, test_name: str) -> str: """Returns test log with change points marked as horizontal lines""" lines = self.__format_log().split("\n") col_widths = self.__column_widths(lines) - indexes = [cp.index for cp in self.__change_points] + indexes = [list(cpg.changes.values())[0].index for cpg in self.__change_points] separators = [] columns = list( OrderedDict.fromkeys(["time", *self.__series.attributes, *self.__series.data]) ) - for cp in self.__change_points: + for cpg in self.__change_points: separator = "" info = "" for col_index, col_name in enumerate(columns): col_width = col_widths[col_index] - change = [c for c in cp.changes if c.metric == col_name] + change = [c for m, c in cpg.changes.items() if m == col_name] if change: - change = change[0] + change = ChangePointSerializer(change[0]) change_percent = change.forward_change_percent() separator += "·" * col_width + " " info += f"{change_percent:+.1f}%".rjust(col_width) + " " diff --git a/otava/series.py b/otava/series.py index cd55159e..5e1f9627 100644 --- a/otava/series.py +++ b/otava/series.py @@ -18,7 +18,6 @@ import logging from dataclasses import dataclass from datetime import datetime, timezone -from itertools import groupby from typing import Any, Dict, Iterable, List, Optional from otava.analysis import ( @@ -28,7 +27,13 @@ compute_change_points_orig, fill_missing, ) -from otava.change_point_divisive.base import ChangePointOtava +from otava.change_point_divisive.base import ( + ChangePoint, + ChangePointGroup, + ChangePoints, + ChangePointsByMetric, + ChangePointsByTime, +) @dataclass @@ -49,7 +54,7 @@ def to_json(self): "window_len": self.window_len, "max_pvalue": self.max_pvalue, "min_magnitude": self.min_magnitude, - "orig_edivisive": self.orig_edivisive + "orig_edivisive": self.orig_edivisive, } @@ -65,90 +70,7 @@ def __init__(self, direction: int = 1, scale: float = 1.0, unit: str = ""): self.unit = "" def to_json(self): - return { - "direction": self.direction, - "scale": self.scale, - "unit": self.unit - } - - -@dataclass -class ChangePointHunter(ChangePointOtava[TTestStats]): - """A change-point for a single metric""" - metric: str - time: int - - def forward_change_percent(self) -> float: - return self.stats.forward_rel_change() * 100.0 - - def backward_change_percent(self) -> float: - return self.stats.backward_rel_change() * 100.0 - - def magnitude(self): - return self.stats.change_magnitude() - - def mean_before(self): - return self.stats.mean_1 - - def mean_after(self): - return self.stats.mean_2 - - def stddev_before(self): - return self.stats.std_1 - - def stddev_after(self): - return self.stats.std_2 - - def pvalue(self): - return self.stats.pvalue - - def to_json(self, rounded=True): - if rounded: - return { - "metric": self.metric, - "index": int(self.index), - "time": self.time, - "forward_change_percent": f"{self.forward_change_percent():.0f}", - "magnitude": f"{self.magnitude():-0f}", - "mean_before": f"{self.mean_before():-0f}", - "stddev_before": f"{self.stddev_before():-0f}", - "mean_after": f"{self.mean_after():-0f}", - "stddev_after": f"{self.stddev_after():-0f}", - "pvalue": f"{self.pvalue():-0f}", - } - - else: - return { - "metric": self.metric, - "index": int(self.index), - "time": self.time, - "forward_change_percent": self.forward_change_percent(), - "magnitude": self.magnitude(), - "mean_before": self.mean_before(), - "stddev_before": self.stddev_before(), - "mean_after": self.mean_after(), - "stddev_after": self.stddev_after(), - "pvalue": self.pvalue(), - } - - -@dataclass -class ChangePointGroup: - """A group of change points on multiple metrics, at the same time""" - - index: int - time: float - prev_time: int - attributes: Dict[str, str] - prev_attributes: Dict[str, str] - changes: List[ChangePointHunter] - - def to_json(self, rounded=False): - return { - "time": self.time, - "attributes": self.attributes, - "changes": [cp.to_json(rounded=rounded) for cp in self.changes], - } + return {"direction": self.direction, "scale": self.scale, "unit": self.unit} class Series: @@ -185,7 +107,7 @@ def __init__( def attributes_at(self, index: int) -> Dict[str, str]: result = {} - for (k, v) in self.attributes.items(): + for k, v in self.attributes.items(): result[k] = v[index] return result @@ -216,13 +138,16 @@ class AnalyzedSeries: __series: Series options: AnalysisOptions - change_points: Dict[str, List[ChangePointHunter]] - change_points_by_time: List[ChangePointGroup] + change_points: Dict[str, ChangePointGroup] + change_points_by_time: ChangePoints change_points_timestamp: Any - def __init__(self, series: Series, options: AnalysisOptions, change_points: Dict[str, ChangePointHunter] = None): + def __init__( + self, series: Series, options: AnalysisOptions, change_points: Dict[str, ChangePoint] = None + ): self.__series = series self.options = options + # record when these change points were calculated self.change_points_timestamp = datetime.now(tz=timezone.utc) self.change_points = None if change_points is not None: @@ -236,7 +161,8 @@ def __init__(self, series: Series, options: AnalysisOptions, change_points: Dict @staticmethod def __compute_change_points( series: Series, options: AnalysisOptions - ) -> Dict[str, List[ChangePointHunter]]: + ) -> (ChangePointsByMetric, ChangePointsByMetric): + # To find change points, go one metric at a time result = {} weak_change_points = {} for metric in series.data.keys(): @@ -249,16 +175,15 @@ def __compute_change_points( values, max_pvalue=options.max_pvalue, ) - tester = TTestSignificanceTester(options.max_pvalue) - intervals = tester.get_intervals(change_points) + result[metric] = [] for c in change_points: - cp_ttest = tester.change_point(c.to_candidate(), values, intervals) - result[metric].append( - ChangePoint( - index=cp_ttest.index, qhat=cp_ttest.qhat, - time=series.time[cp_ttest.index], metric=metric, stats=cp_ttest.stats - ) + c.metric = metric + cpg = ChangePointGroup( + time=series.time[c.index], + attributes=series.attributes_at(c.index), + changes={metric: c}, ) + result[metric].append(cpg) else: change_points, weak_cps = compute_change_points( values, @@ -267,43 +192,29 @@ def __compute_change_points( min_magnitude=options.min_magnitude, ) for c in weak_cps: - weak_change_points[metric].append( - ChangePointHunter( - index=c.index, qhat=0.0, time=series.time[c.index], metric=metric, stats=c.stats - ) + c.metric = metric + cpg = ChangePointGroup( + time=series.time[c.index], + attributes=series.attributes_at(c.index), + changes={metric: c}, ) + weak_change_points[metric].append(cpg) for c in change_points: - result[metric].append( - ChangePointHunter( - index=c.index, qhat=0.0, time=series.time[c.index], metric=metric, stats=c.stats - ) + c.metric = metric + cpg = ChangePointGroup( + time=series.time[c.index], + attributes=series.attributes_at(c.index), + changes={metric: c}, ) - # If you got an exception and are wondering about the next row... - # weak_cps is an optimization which you can ignore - return result, weak_change_points + result[metric].append(cpg) + + return ChangePointsByMetric(result), ChangePointsByMetric(weak_change_points) @staticmethod def __group_change_points_by_time( - series: Series, change_points: Dict[str, List[ChangePointHunter]] - ) -> List[ChangePointGroup]: - changes: List[ChangePointHunter] = [] - for metric in change_points.keys(): - changes += change_points[metric] - - changes.sort(key=lambda c: c.index) - points = [] - for k, g in groupby(changes, key=lambda c: c.index): - cp = ChangePointGroup( - index=k, - time=series.time[k], - prev_time=series.time[k - 1], - attributes=series.attributes_at(k), - prev_attributes=series.attributes_at(k - 1), - changes=list(g), - ) - points.append(cp) - - return points + series: Series, change_points: ChangePoints + ) -> ChangePointsByTime: + return ChangePointsByTime(change_points) def get_stable_range(self, metric: str, index: int) -> (int, int): """ @@ -316,13 +227,13 @@ def get_stable_range(self, metric: str, index: int) -> (int, int): It follows that there are no change points between A and B. """ begin = 0 - for cp in self.change_points[metric]: + for cp in self.change_points.get_change_points_for_metric(metric): if cp.index > index: break begin = cp.index end = len(self.time()) - for cp in reversed(self.change_points[metric]): + for cp in reversed(self.change_points.get_change_points_for_metric(metric)): if cp.index <= index: break end = cp.index @@ -347,7 +258,9 @@ def _validate_append(self, time, new_data, attributes): max_time = max(self.__series.time) for t in time: if t <= max_time: - return ValueError("time must be monotonously increasing if you use append() time={}".format(time)) + return ValueError( + "time must be monotonously increasing if you use append() time={}".format(time) + ) return None @@ -377,7 +290,7 @@ def append(self, time, new_data, attributes): for metric in self.__series.data.keys(): if metric not in new_data: - weak_change_points[metric] = self.weak_change_points[metric] + weak_change_points[metric] = self.weak_change_points.select_metrics(metric) continue change_points, weak_cps = compute_change_points( @@ -386,32 +299,40 @@ def append(self, time, new_data, attributes): max_pvalue=self.options.max_pvalue, min_magnitude=self.options.min_magnitude, new_data=len(new_data[metric]), - old_weak_cp=self.weak_change_points.get(metric, []) + old_weak_cp=self.weak_change_points.get_change_points_for_metric(metric), ) - result[metric] = [] + if metric not in result: + result[metric] = [] for c in change_points: result[metric].append( - ChangePointHunter( - index=c.index, qhat=0.0, time=self.__series.time[c.index], metric=metric, stats=c.stats + ChangePointGroup( + time=self.__series.time[c.index], + changes={metric: c}, + attributes=self.__series.attributes_at(c.index), ) ) - weak_change_points[metric] = [] + if metric not in weak_change_points: + weak_change_points[metric] = [] for c in weak_cps: weak_change_points[metric].append( - ChangePointHunter( - index=c.index, qhat=0.0, time=self.__series.time[c.index], metric=metric, stats=c.stats + ChangePointGroup( + time=self.__series.time[c.index], + changes={metric: c}, + attributes=self.__series.attributes_at(c.index), ) ) + # TODO: Remove this. It should not be a requirement that metrics have the same history. fill_missing(self.__series.data[metric]) - # If some metrics didn't participate in this round, we still keep them, but update the ones - # We did recompute - for metric in result.keys(): - self.change_points[metric] = result[metric] - for metric in weak_change_points.keys(): - self.weak_change_points[metric] = weak_change_points[metric] - self.change_points_by_time = self.__group_change_points_by_time(self.__series, self.change_points) - return result, weak_change_points + r = ChangePointsByMetric(result) + w = ChangePointsByMetric(weak_change_points) + # print("#", self.change_points.change_points) + # print("¤", r.change_points) + # print("%", w.change_points) + # r has a subset of all metrics, so can't just set change_points to r + for metric, cpglist in r.change_points.items(): + self.change_points.change_points[metric] = cpglist + return r, w def test_name(self) -> str: return self.__series.test_name @@ -445,12 +366,12 @@ def metric(self, name: str) -> Metric: def to_json(self): change_points_json = {} - for metric, cps in self.change_points.items(): - change_points_json[metric] = [cp.to_json(rounded=False) for cp in cps] + for cps in self.change_points: + change_points_json = [cp.to_json(rounded=False) for cp in cps] weak_change_points_json = {} - for metric, cps in self.weak_change_points.items(): - weak_change_points_json[metric] = [cp.to_json(rounded=False) for cp in cps] + for cps in self.weak_change_points: + weak_change_points_json = [cp.to_json(rounded=False) for cp in cps] data_json = {} for metric, datapoints in self.__series.data.items(): @@ -466,7 +387,7 @@ def to_json(self): "attributes": self.__series.attributes, "data": self.__series.data, "change_points": change_points_json, - "weak_change_points": weak_change_points_json + "weak_change_points": weak_change_points_json, } @classmethod @@ -482,7 +403,7 @@ def from_json(cls, analyzed_json): analyzed_json["time"], new_metrics, analyzed_json["data"], - analyzed_json["attributes"] + analyzed_json["attributes"], ) new_options = AnalysisOptions() @@ -503,9 +424,7 @@ def from_json(cls, analyzed_json): pvalue=cp["pvalue"], ) new_list.append( - ChangePointHunter( - index=cp["index"], time=cp["time"], metric=cp["metric"], stats=stat - ) + ChangePoint(index=cp["index"], time=cp["time"], metric=cp["metric"], stats=stat) ) new_change_points[metric] = new_list @@ -521,9 +440,7 @@ def from_json(cls, analyzed_json): pvalue=cp["pvalue"], ) new_list.append( - ChangePointHunter( - index=cp["index"], time=cp["time"], metric=cp["metric"], stats=stat - ) + ChangePoint(index=cp["index"], time=cp["time"], metric=cp["metric"], stats=stat) ) new_weak_change_points[metric] = new_list @@ -532,6 +449,8 @@ def from_json(cls, analyzed_json): if "change_points_timestamp" in analyzed_json.keys(): analyzed_series.change_points_timestamp = analyzed_json["change_points_timestamp"] - analyzed_series.change_points_by_time = AnalyzedSeries.__group_change_points_by_time(analyzed_series.__series, analyzed_series.change_points) + analyzed_series.change_points_by_time = AnalyzedSeries.__group_change_points_by_time( + analyzed_series.__series, analyzed_series.change_points + ) return analyzed_series diff --git a/otava/slack.py b/otava/slack.py index a0d4bc3d..441f0836 100644 --- a/otava/slack.py +++ b/otava/slack.py @@ -23,8 +23,9 @@ from pytz import UTC from slack_sdk import WebClient +from otava.change_point_divisive.base import ChangePointGroup, ChangePointSerializer from otava.data_selector import DataSelector -from otava.series import AnalyzedSeries, ChangePointGroup +from otava.series import AnalyzedSeries @dataclass @@ -202,8 +203,9 @@ def __dates_change_points_summary(self, test_changes: Dict[str, ChangePointGroup for test_name, group in test_changes.items(): fields.append(f"*{test_name}*") summary = "" - for change in group.changes: - change_percent = change.forward_change_percent() + for metric, change in group.changes.items(): + c = ChangePointSerializer(change) + change_percent = c.forward_change_percent() change_emoji = self.__get_change_emoji(test_name, change) if isinf(change_percent): report_percent = change_percent @@ -228,7 +230,8 @@ def __dates_change_points_summary(self, test_changes: Dict[str, ChangePointGroup def __get_change_emoji(self, test_name, change): metric_direction = self.test_analyzed_series[test_name].metric(change.metric).direction - regression = metric_direction * change.forward_change_percent() + c = ChangePointSerializer(change) + regression = metric_direction * c.forward_change_percent() if regression >= 0: return ":large_blue_circle:" else: diff --git a/tests/change_point_divisive_test.py b/tests/change_point_divisive_test.py index 950d982a..45d44318 100644 --- a/tests/change_point_divisive_test.py +++ b/tests/change_point_divisive_test.py @@ -19,7 +19,7 @@ import pytest from otava.analysis import TTestSignificanceTester, TTestStats -from otava.change_point_divisive.base import ChangePointOtava +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 @@ -124,9 +124,9 @@ def test_get_intervals_requires_sorted_change_points(): # Sorted change points should work sorted_cps = [ - ChangePointOtava(index=5, qhat=1.0, stats=stats), - ChangePointOtava(index=10, qhat=1.0, stats=stats), - ChangePointOtava(index=15, qhat=1.0, stats=stats), + 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 @@ -137,9 +137,9 @@ def test_get_intervals_requires_sorted_change_points(): # Unsorted change points should raise AssertionError unsorted_cps = [ - ChangePointOtava(index=10, qhat=1.0, stats=stats), - ChangePointOtava(index=5, qhat=1.0, stats=stats), - ChangePointOtava(index=15, qhat=1.0, stats=stats), + 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) diff --git a/tests/report_test.py b/tests/report_test.py index a220eb55..6e8b4836 100644 --- a/tests/report_test.py +++ b/tests/report_test.py @@ -86,8 +86,7 @@ def test_json_report(report): 'metric': 'series2', 'pvalue': '0.000000', 'stddev_after': '0.026954', - 'stddev_before': '0.011180', - 'time': 4}], + 'stddev_before': '0.011180'}], 'time': 4}, {'attributes': {}, 'changes': [{'forward_change_percent': '-49', @@ -98,8 +97,10 @@ def test_json_report(report): 'metric': 'series1', 'pvalue': '0.000000', 'stddev_after': '0.025768', - 'stddev_before': '0.067495', - 'time': 6}], + 'stddev_before': '0.067495'}], 'time': 6}]} assert isinstance(obj, dict) + from pprint import pprint + pprint(obj) + pprint(expected) assert obj == expected diff --git a/tests/series_test.py b/tests/series_test.py index 94fbe544..51112384 100644 --- a/tests/series_test.py +++ b/tests/series_test.py @@ -20,6 +20,7 @@ import pytest +from otava.change_point_divisive.base import ChangePointSerializer from otava.series import AnalysisOptions, Metric, Series @@ -36,12 +37,66 @@ def test_change_point_detection(): attributes={}, ) - change_points = test.analyze().change_points_by_time - assert len(change_points) == 2 - assert change_points[0].index == 4 - assert change_points[0].changes[0].metric == "series2" - assert change_points[1].index == 6 - assert change_points[1].changes[0].metric == "series1" + cps = test.analyze().change_points_by_time + assert len(cps) == 2 + assert cps.change_points[0].time == 4 + assert cps.change_points[0].changes["series2"].metric == "series2" + assert cps.change_points[1].time == 6 + assert cps.change_points[1].changes["series1"].metric == "series1" + + +def test_change_point_detection_many(): + series_3 = [ + 1, + 1, + 1, + 1, + 1, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + 9, + 9, + 9, + 9, + 9, + 9, + 9, + 9, + 3, + 3, + 3, + 3, + 3, + 3, + 3, + 3, + 3, + ] + time = list(range(len(series_3))) + test = Series( + "test", + branch=None, + time=time, + metrics={"series3": Metric(1, 1.0)}, + data={"series3": series_3}, + attributes={}, + ) + + options = AnalysisOptions() + options.min_magnitude = 0.0 + options.max_pvalue = 0.05 + analyzed_series = test.analyze(options) + assert len(list(analyzed_series.change_points)) == 3 + cps_by_time = analyzed_series.change_points_by_time + assert len(cps_by_time.change_points) == 3 + assert analyzed_series.change_points[0].time == 5 + assert "series3" in analyzed_series.change_points[0].changes def test_change_point_min_magnitude(): @@ -59,16 +114,16 @@ def test_change_point_min_magnitude(): options = AnalysisOptions() options.min_magnitude = 0.2 - change_points = test.analyze(options).change_points_by_time - assert len(change_points) == 1 - assert change_points[0].index == 6 - assert change_points[0].changes[0].metric == "series1" + cps = test.analyze(options).change_points_by_time + assert len(cps) == 1 + assert cps.change_points[0].time == 6 + assert "series1" in cps[0].changes - for change_point in change_points: - for change in change_point.changes: - assert ( - change.magnitude() >= options.min_magnitude - ), f"All change points must have magnitude greater than {options.min_magnitude}" + for change_point in cps: + for metric, change in change_point.changes.items(): + assert ChangePointSerializer(change).magnitude() >= options.min_magnitude, ( + f"All change points must have magnitude greater than {options.min_magnitude}" + ) # Divide by zero is only a RuntimeWarning, but for testing we want to make sure it's a failure @@ -90,7 +145,7 @@ def test_div_by_zero(): cpjson = analyzed_series.to_json() assert cpjson assert len(change_points) == 2 - assert change_points[0].index == 3 + assert change_points[0].time == 3 def test_change_point_detection_performance(): @@ -151,20 +206,22 @@ def test_incremental_otava(): ) analyzed_series = test.analyze() - analyzed_series.append(time=[len(time)], new_data={"series1": [0.5], "series2": [1.97]}, attributes={}) + analyzed_series.append( + time=[len(time)], new_data={"series1": [0.5], "series2": [1.97]}, attributes={} + ) change_points = analyzed_series.change_points - assert [c.index for c in change_points["series1"]] == [6] - assert [c.index for c in change_points["series2"]] == [4] + assert [c.index for c in change_points.get_change_points_for_metric("series1")] == [6] + assert [c.index for c in change_points.get_change_points_for_metric("series2")] == [4] analyzed_series.append(time=[len(time)], new_data={"series1": [0.51]}, attributes={}) change_points = analyzed_series.change_points - assert [c.index for c in change_points["series1"]] == [6] - assert [c.index for c in change_points["series2"]] == [4] + assert [c.index for c in change_points.get_change_points_for_metric("series1")] == [6] + assert [c.index for c in change_points.get_change_points_for_metric("series2")] == [4] analyzed_series.append(time=[len(time)], new_data={"series2": [33.33, 46.46]}, attributes={}) change_points = analyzed_series.change_points - assert [c.index for c in change_points["series1"]] == [6] - assert [c.index for c in change_points["series2"]] == [4, 12] + assert [c.index for c in change_points.get_change_points_for_metric("series1")] == [6] + assert [c.index for c in change_points.get_change_points_for_metric("series2")] == [4, 12] def test_validate(): @@ -190,13 +247,19 @@ def test_validate(): analyzed_series_fail = test_fail.analyze() analyzed_series_fail.change_points = None - err = analyzed_series_fail._validate_append(time=[len(time)], new_data={"series1": [0.51]}, attributes={}) + err = analyzed_series_fail._validate_append( + time=[len(time)], new_data={"series1": [0.51]}, attributes={} + ) assert isinstance(err, RuntimeError) analyzed_series = test.analyze() - analyzed_series.append(time=[len(time)], new_data={"series1": [0.5], "series2": [1.97]}, attributes={}) + analyzed_series.append( + time=[len(time)], new_data={"series1": [0.5], "series2": [1.97]}, attributes={} + ) - err = analyzed_series._validate_append(time=[len(time)], new_data={"series1": [0.51]}, attributes={}) + err = analyzed_series._validate_append( + time=[len(time)], new_data={"series1": [0.51]}, attributes={} + ) assert err is None err = analyzed_series._validate_append(time=[5], new_data={"series1": [0.51]}, attributes={}) @@ -220,7 +283,9 @@ def test_can_append(): ) analyzed_series = test.analyze() - analyzed_series.append(time=[len(time)], new_data={"series1": [0.5], "series2": [1.97]}, attributes={}) + analyzed_series.append( + time=[len(time)], new_data={"series1": [0.5], "series2": [1.97]}, attributes={} + ) can = analyzed_series.can_append(time=[len(time)], new_data={"series1": [0.51]}, attributes={}) assert can From 6a2418210a39c8f4cbf1003d69cb24edc2b315f9 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Sun, 31 May 2026 12:58:15 +0300 Subject: [PATCH 03/27] Refactor common code between the significance testers This moves stats and functionality up towards parent classes so that generic stats like mean are always computed for all variants. In fact TTestStat is now an empty class, it's functionality fully absorbed by the parent. (But note that the class name/type itself carries information about the pvalue. --- otava/analysis.py | 112 ++------------- otava/change_point_divisive/base.py | 129 +++++++++++++++++- .../significance_test.py | 11 +- 3 files changed, 148 insertions(+), 104 deletions(-) diff --git a/otava/analysis.py b/otava/analysis.py index 84c69c2c..08ed0af9 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -17,7 +17,7 @@ import copy from dataclasses import dataclass -from typing import List, Optional, Sequence, SupportsFloat, Tuple +from typing import List, Optional, Sequence, SupportsFloat, Tuple, cast import numpy as np from scipy.stats import ttest_ind_from_stats @@ -43,59 +43,8 @@ class TTestStats(BaseStats): Keeps statistics of two series of data and the probability both series have the same distribution. """ - - mean_1: float - mean_2: float - std_1: float - std_2: float - - def forward_rel_change(self, value_if_nan=0): - """Relative change from left to right""" - if self.mean_1 == 0: - return value_if_nan - - return self.mean_2 / self.mean_1 - 1.0 - - def backward_rel_change(self, value_if_nan=0): - """Relative change from right to left""" - if self.mean_2 == 0: - return value_if_nan - - return self.mean_1 / self.mean_2 - 1.0 - - def forward_change_percent(self) -> float: - return self.forward_rel_change() * 100.0 - - def backward_change_percent(self) -> float: - return self.backward_rel_change() * 100.0 - - def change_magnitude(self): - """Maximum of absolutes of rel_change and rel_change_reversed""" - return max(abs(self.forward_rel_change()), abs(self.backward_rel_change())) - - def mean_before(self): - return self.mean_1 - - def mean_after(self): - return self.mean_2 - - def stddev_before(self): - return self.std_1 - - def stddev_after(self): - return self.std_2 - - def to_json(self): - return { - "forward_change_percent": f"{self.forward_change_percent():-0f}", - "magnitude": f"{self.change_magnitude():-0f}", - "mean_before": f"{self.mean_before():-0f}", - "stddev_before": f"{self.stddev_before():-0f}", - "mean_after": f"{self.mean_after():-0f}", - "stddev_after": f"{self.stddev_after():-0f}", - "pvalue": f"{self.pvalue:-0f}", - } - + # TODO + pass # Generic Change Point List GenCPList = List[ChangePoint[GenericStats]] @@ -105,6 +54,7 @@ def to_json(self): TtestCPList = List[ChangePoint[TTestStats]] +# TODO: Move to change_point_divisive.significance_test class TTestSignificanceTester(SignificanceTester): """ Uses two-sided Student's T-test to decide if a candidate change point @@ -114,21 +64,15 @@ class TTestSignificanceTester(SignificanceTester): """ def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat]) -> TTestStats: - if len(left) == 0 or len(right) == 0: - raise ValueError - - mean_l = np.mean(left) - mean_r = np.mean(right) - std_l = np.std(left) if len(left) >= 2 else 0.0 - std_r = np.std(right) if len(right) >= 2 else 0.0 - + base_stats = super().compare(left, right) if len(left) + len(right) > 2: (_, p) = ttest_ind_from_stats( - mean_l, std_l, len(left), mean_r, std_r, len(right), alternative="two-sided" + base_stats.mean_1, base_stats.std_1, len(left), base_stats.mean_2, base_stats.std_2, len(right), alternative="two-sided" ) else: p = 1.0 - return TTestStats(mean_1=mean_l, mean_2=mean_r, std_1=std_l, std_2=std_r, pvalue=p) + + return TTestStats(pvalue=p, mean_1=base_stats.mean_1, mean_2=base_stats.mean_2, std_1=base_stats.std_1, std_2=base_stats.std_2) def change_point( self, @@ -136,43 +80,7 @@ def change_point( series: Sequence[SupportsFloat], intervals: List[slice], ) -> ChangePoint[TTestStats]: - """ - Computes properties of the change point if the Candidate Change Point based on the provided intervals. - - The method works in both steps of the algorithm: - 1. Split step: - if the candidate is a new potential change point, i.e., its index is inside any interval, then - we split the interval by the candidate's index to get left and right subseries. - 2. Merge step: - if the candidate is an existing change point, i.e., it matches the end of two intervals, then - it's a potential weak change point, and we don't need to split the intervals anymore (just take - both intervals as left and right subseries). - - """ - for i, interval in enumerate(intervals): - if interval.stop == candidate.index: - # Merge step - left_interval = interval - right_interval = intervals[i + 1] - break - elif (interval.start is None or interval.start < candidate.index) and (interval.stop is None or candidate.index < interval.stop): - # Split step - # Note: handles slices with omitted indexes: - # - # array[0 : i] == array[:i] == array[slice(None, i)] == array[slice(0, i)], - # i.e., interval.start == None and interval.start == 0 are equivalent. - # - # array[i: len(array)] == array[i:] == array[slice(i, None)] == array[slice(i, len(array))], - # i.e., interval.stop == None and interval.stop == len(array) are equivalent. - left_interval = slice(interval.start, candidate.index) - right_interval = slice(candidate.index, interval.stop) - break - else: - raise ValueError( - f"Candidate Change Point at index={candidate.index} doesn't correspond to any interval in {intervals}." - ) - left = series[left_interval] - right = series[right_interval] + left, right = self.get_sides(candidate, series, intervals) stats = self.compare(left, right) return ChangePoint.from_candidate(candidate, stats) @@ -181,6 +89,8 @@ def fill_missing(data: Sequence[SupportsFloat]): """ Forward-fills None occurrences with nearest previous non-None values. Initial None values are back-filled with the nearest future non-None value. + + TODO: Remove this. """ prev = None for i in range(len(data)): diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 275a61fa..e5477c88 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -48,8 +48,9 @@ from collections import OrderedDict from dataclasses import dataclass, fields from datetime import UTC, datetime -from typing import Dict, Generic, List, Optional, TypeVar +from typing import Dict, Generic, List, Optional, TypeVar, Sequence, SupportsFloat, Any +import numpy as np from numpy.typing import NDArray @@ -67,7 +68,83 @@ class BaseStats: # The pvalue for this change point. Exact value depends on the algorithm that was used. pvalue: float + mean_1: float + mean_2: float + std_1: float + std_2: float + def __init__(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat], pvalue=None) -> Any: + """ + Basic statsistics about the left and right side, and the change between them. + + Calculate basic statistics about the left and right sides of a change point, such as mean + standard deviation. p-value depeds on the significance test used, so we cannot know or compute + it here, but if the caller knows p already, they can supply it as argument. + """ + self.calculate_base_stats(left, right, pvalue) + + def calculate_base_stats(self, left, right, pvalue=None): + if pvalue is not None and pvalue >= 0.0 and pvalue <= 1.0: + self.pvalue = pvalue + else: + self.pvalue = 1.0 + + if len(left) == 0 or len(right) == 0: + raise ValueError + + self.mean_1 = np.mean(left) + self.mean_2 = np.mean(right) + self.std_1 = np.std(left) if len(left) >= 2 else 0.0 + self.std_2 = np.std(right) if len(right) >= 2 else 0.0 + + return self + + def forward_rel_change(self, value_if_nan=0): + """Relative change from left to right""" + if self.mean_1 == 0: + return value_if_nan + + return self.mean_2 / self.mean_1 - 1.0 + + def backward_rel_change(self, value_if_nan=0): + """Relative change from right to left""" + if self.mean_2 == 0: + return value_if_nan + + return self.mean_1 / self.mean_2 - 1.0 + + def forward_change_percent(self) -> float: + return self.forward_rel_change() * 100.0 + + def backward_change_percent(self) -> float: + return self.backward_rel_change() * 100.0 + + def change_magnitude(self): + """Maximum of absolutes of rel_change and rel_change_reversed""" + return max(abs(self.forward_rel_change()), abs(self.backward_rel_change())) + + def mean_before(self): + return self.mean_1 + + def mean_after(self): + return self.mean_2 + + def stddev_before(self): + return self.std_1 + + def stddev_after(self): + return self.std_2 + + def to_json(self): + return { + "forward_change_percent": f"{self.forward_change_percent():-0f}", + "magnitude": f"{self.change_magnitude():-0f}", + "mean_before": f"{self.mean_before():-0f}", + "stddev_before": f"{self.stddev_before():-0f}", + "mean_after": f"{self.mean_after():-0f}", + "stddev_after": f"{self.stddev_after():-0f}", + "pvalue": f"{self.pvalue:-0f}", + } # Abstract variable type for statistics, corresponds to BaseStats class and its subclasses. GenericStats = TypeVar("GenericStats", bound=BaseStats) @@ -520,6 +597,55 @@ class SignificanceTester(Generic[GenericStats]): def __init__(self, max_pvalue: float): self.max_pvalue = max_pvalue + def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat], p: float = None) -> GenericStats: + if len(left) == 0 or len(right) == 0: + raise ValueError + return BaseStats(left, right, p) + + + def get_sides( + self, candidate: CandidateChangePoint, series: Sequence[SupportsFloat], intervals: List[slice] + ) -> (Sequence, Sequence): + """ + Computes properties of the change point if the Candidate Change Point based on the provided intervals. + + The method works in both steps of the algorithm: + 1. Split step: + if the candidate is a new potential change point, i.e., its index is inside any interval, then + we split the interval by the candidate's index to get left and right subseries. + 2. Merge step: + if the candidate is an existing change point, i.e., it matches the end of two intervals, then + it's a potential weak change point, and we don't need to split the intervals anymore (just take + both intervals as left and right subseries). + + """ + for i, interval in enumerate(intervals): + if interval.stop == candidate.index: + # Merge step + left_interval = interval + right_interval = intervals[i + 1] + break + elif (interval.start is None or interval.start < candidate.index) and (interval.stop is None or candidate.index < interval.stop): + # Split step + # Note: handles slices with omitted indexes: + # + # array[0 : i] == array[:i] == array[slice(None, i)] == array[slice(0, i)], + # i.e., interval.start == None and interval.start == 0 are equivalent. + # + # array[i: len(array)] == array[i:] == array[slice(i, None)] == array[slice(i, len(array))], + # i.e., interval.stop == None and interval.stop == len(array) are equivalent. + left_interval = slice(interval.start, candidate.index) + right_interval = slice(candidate.index, interval.stop) + break + else: + raise ValueError( + f"Candidate Change Point at index={candidate.index} doesn't correspond to any interval in {intervals}." + ) + left = series[left_interval] + right = series[right_interval] + return left, right + + def get_intervals(self, change_points: List[ChangePoint[GenericStats]]) -> List[slice]: """Returns list of slices of the series. Change points must be sorted by index.""" assert all( @@ -545,7 +671,6 @@ def change_point( """Computes stats for a change point candidate and wraps it into ChangePoint class""" ... - class Calculator: """Abstract class for calculator. Calculator provides an interface to get best change point candidate""" diff --git a/otava/change_point_divisive/significance_test.py b/otava/change_point_divisive/significance_test.py index 7e348c71..29087a3f 100644 --- a/otava/change_point_divisive/significance_test.py +++ b/otava/change_point_divisive/significance_test.py @@ -68,10 +68,19 @@ def change_point(self, candidate: CandidateChangePoint, series: NDArray, interva # 2. Estimate p-value extreme_qhat_perm = np.sum(qhats >= candidate.qhat) pval = extreme_qhat_perm / (self.permutations + 1) + + # 3. Add general statistics together with the above + left, right = self.get_sides(candidate, series, intervals) + generic_stats = self.compare(left, right, pval) + stats = PermutationStats( pvalue=pval, permuted_qhats=qhats, extreme_qhat_perm=extreme_qhat_perm, - n_perm=self.permutations + n_perm=self.permutations, + mean_1=generic_stats.mean_1, + mean_2=generic_stats.mean_2, + std_1=generic_stats.std_1, + std_2=generic_stats.std_2, ) return ChangePoint.from_candidate(candidate, stats) From 2aca0dacd42463981479ef6e2a3470b69b550ab3 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Tue, 2 Jun 2026 19:47:11 +0300 Subject: [PATCH 04/27] ruff/flake/idk --- otava/analysis.py | 3 +-- otava/series.py | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/otava/analysis.py b/otava/analysis.py index 08ed0af9..4f8854b3 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -17,9 +17,8 @@ import copy from dataclasses import dataclass -from typing import List, Optional, Sequence, SupportsFloat, Tuple, cast +from typing import List, Optional, Sequence, SupportsFloat, Tuple -import numpy as np from scipy.stats import ttest_ind_from_stats from otava.change_point_divisive.base import ( diff --git a/otava/series.py b/otava/series.py index 5e1f9627..729bb142 100644 --- a/otava/series.py +++ b/otava/series.py @@ -21,7 +21,6 @@ from typing import Any, Dict, Iterable, List, Optional from otava.analysis import ( - TTestSignificanceTester, TTestStats, compute_change_points, compute_change_points_orig, From 20e83888ddc9476b14f71189b9ca2d7f5edf2ce0 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Thu, 4 Jun 2026 14:36:38 +0300 Subject: [PATCH 05/27] format --- .github/workflows/python-app.yml | 4 ++-- otava/analysis.py | 2 +- otava/change_point_divisive/base.py | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 1c035842..090d5226 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -52,8 +52,8 @@ jobs: - name: Install the project run: uv sync --all-extras --dev - - name: Install tox - run: uv tool install tox --with tox-uv + # - name: Install tox + # run: uv tool install tox --with tox-uv - name: Run tox run: tox -e py diff --git a/otava/analysis.py b/otava/analysis.py index 4f8854b3..b9cd9f50 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -43,7 +43,7 @@ class TTestStats(BaseStats): have the same distribution. """ # TODO - pass + # Generic Change Point List GenCPList = List[ChangePoint[GenericStats]] diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index e5477c88..9a6f4f06 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -48,7 +48,7 @@ from collections import OrderedDict from dataclasses import dataclass, fields from datetime import UTC, datetime -from typing import Dict, Generic, List, Optional, TypeVar, Sequence, SupportsFloat, Any +from typing import Any, Dict, Generic, List, Optional, Sequence, SupportsFloat, TypeVar import numpy as np from numpy.typing import NDArray @@ -146,6 +146,7 @@ def to_json(self): "pvalue": f"{self.pvalue:-0f}", } + # Abstract variable type for statistics, corresponds to BaseStats class and its subclasses. GenericStats = TypeVar("GenericStats", bound=BaseStats) @@ -602,10 +603,9 @@ def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat], raise ValueError return BaseStats(left, right, p) - def get_sides( - self, candidate: CandidateChangePoint, series: Sequence[SupportsFloat], intervals: List[slice] - ) -> (Sequence, Sequence): + self, candidate: CandidateChangePoint, series: Sequence[SupportsFloat], intervals: List[slice] + ) -> (Sequence, Sequence): """ Computes properties of the change point if the Candidate Change Point based on the provided intervals. @@ -645,7 +645,6 @@ def get_sides( right = series[right_interval] return left, right - def get_intervals(self, change_points: List[ChangePoint[GenericStats]]) -> List[slice]: """Returns list of slices of the series. Change points must be sorted by index.""" assert all( @@ -671,6 +670,7 @@ def change_point( """Computes stats for a change point candidate and wraps it into ChangePoint class""" ... + class Calculator: """Abstract class for calculator. Calculator provides an interface to get best change point candidate""" From 05a4e2be2782c45dab9253f046b9f701fb10bb63 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Thu, 4 Jun 2026 14:52:15 +0300 Subject: [PATCH 06/27] Re-enable tox alsso in gh actions --- .github/workflows/python-app.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 090d5226..1c035842 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -52,8 +52,8 @@ jobs: - name: Install the project run: uv sync --all-extras --dev - # - name: Install tox - # run: uv tool install tox --with tox-uv + - name: Install tox + run: uv tool install tox --with tox-uv - name: Run tox run: tox -e py From bb84ad9a4ca38cf3c15e1e430745c9ef1daa224f Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Thu, 4 Jun 2026 16:30:26 +0300 Subject: [PATCH 07/27] fix: timezone in 3.10 compatible way --- otava/series.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otava/series.py b/otava/series.py index 729bb142..8f384417 100644 --- a/otava/series.py +++ b/otava/series.py @@ -147,7 +147,7 @@ def __init__( self.__series = series self.options = options # record when these change points were calculated - self.change_points_timestamp = datetime.now(tz=timezone.utc) + self.change_points_timestamp = datetime.now(timezone.utc) self.change_points = None if change_points is not None: self.change_points = change_points From cffbfb7c5330e39df972a955d6992abeaaf178b9 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Thu, 4 Jun 2026 16:36:17 +0300 Subject: [PATCH 08/27] bw compat: must use timezone.UTC --- otava/change_point_divisive/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 9a6f4f06..562b1d0f 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -47,7 +47,7 @@ from collections import OrderedDict from dataclasses import dataclass, fields -from datetime import UTC, datetime +from datetime import datetime, timezone from typing import Any, Dict, Generic, List, Optional, Sequence, SupportsFloat, TypeVar import numpy as np @@ -293,7 +293,7 @@ def commit(self, idx: int): return self.attribute_at(idx).get("commit") def datetime(self): - return datetime.fromtimestamp(self.time, UTC) + return datetime.fromtimestamp(self.time, timezone.UTC) def select_metrics(self, m: list[str] | str): if not isinstance(m, list): From 55665730201f6da297ffe0f5d91770e6f87bef2f Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Sun, 7 Jun 2026 03:40:09 +0300 Subject: [PATCH 09/27] Simplify __compute_change_points() by using ChangePoints earlier --- otava/change_point_divisive/base.py | 24 +++++++++++++++++++----- otava/series.py | 15 ++++++--------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 562b1d0f..5d0b674a 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -364,6 +364,14 @@ def __init__(self, cps=None): ) self.change_points = cps + def by_time(self) -> ChangePointsByTime: + cps = ChangePointsByTime() + cps.change_points = self.change_points + return cps + + def by_metric(self) -> ChangePointsByMetric: + return self.pivot() + def append(self, cpg: ChangePointGroup): if not isinstance(cpg, ChangePointGroup): raise TypeError("ChangePoints.append() takes as argument one ChangePointGroup.") @@ -449,6 +457,14 @@ def at_commit(self, sha: str): return row raise LookupError(sha) + def pivot(self): + """ + Return the same object as ChangePointsByMetric. + """ + raise NotImplementedError() + + +class ChangePointsByTime(ChangePoints): def pivot(self): """ Return the same object as ChangePointsByMetric. @@ -461,13 +477,9 @@ def pivot(self): by_metric.append(row) -class ChangePointsByTime(ChangePoints): - pass - - class ChangePointsByMetric(ChangePoints): """ - Provides same interface as ChangePoints, but internally stores with metric first. + Provides same interface as ChangePointsByTime, but internally stores with metric first. """ def __init__(self, cps=None): @@ -517,6 +529,8 @@ def append(self, cpg: ChangePointGroup): if not isinstance(cpg, ChangePointGroup): raise TypeError("ChangePoints.append() takes as argument one ChangePointGroup.") for metric in cpg.metrics(): + if metric not in self.change_points: + self.change_points[metric] = [] self.change_points[metric].append(cpg.select_metrics(metric)) def extend(self, cps): diff --git a/otava/series.py b/otava/series.py index 8f384417..7f96ebce 100644 --- a/otava/series.py +++ b/otava/series.py @@ -162,11 +162,9 @@ def __compute_change_points( series: Series, options: AnalysisOptions ) -> (ChangePointsByMetric, ChangePointsByMetric): # To find change points, go one metric at a time - result = {} - weak_change_points = {} + result = ChangePointsByMetric() + weak_change_points = ChangePointsByMetric() for metric in series.data.keys(): - result[metric] = [] - weak_change_points[metric] = [] values = series.data[metric].copy() fill_missing(values) if options.orig_edivisive: @@ -174,7 +172,6 @@ def __compute_change_points( values, max_pvalue=options.max_pvalue, ) - result[metric] = [] for c in change_points: c.metric = metric cpg = ChangePointGroup( @@ -182,7 +179,7 @@ def __compute_change_points( attributes=series.attributes_at(c.index), changes={metric: c}, ) - result[metric].append(cpg) + result.append(cpg) else: change_points, weak_cps = compute_change_points( values, @@ -197,7 +194,7 @@ def __compute_change_points( attributes=series.attributes_at(c.index), changes={metric: c}, ) - weak_change_points[metric].append(cpg) + weak_change_points.append(cpg) for c in change_points: c.metric = metric cpg = ChangePointGroup( @@ -205,9 +202,9 @@ def __compute_change_points( attributes=series.attributes_at(c.index), changes={metric: c}, ) - result[metric].append(cpg) + result.append(cpg) - return ChangePointsByMetric(result), ChangePointsByMetric(weak_change_points) + return result, weak_change_points @staticmethod def __group_change_points_by_time( From 553c8676851b9be2383a26bc682ec11b75418884 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 8 Jun 2026 02:59:19 +0300 Subject: [PATCH 10/27] Simplify ChangePoints() constructor Create separate copy(), from_dict(), from_list() constructors Constructors now just create an empty container Add tests for unified ChangePoint classes; fix many bugs Also move BaseStats compute logic into a calculate() staticmethod so copy() is a plain replace(). --- otava/analysis.py | 1 - otava/change_point_divisive/base.py | 232 ++++---- .../significance_test.py | 6 +- otava/series.py | 6 +- tests/change_point_classes_test.py | 515 ++++++++++++++++++ 5 files changed, 663 insertions(+), 97 deletions(-) create mode 100644 tests/change_point_classes_test.py diff --git a/otava/analysis.py b/otava/analysis.py index b9cd9f50..cee0dcf6 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -42,7 +42,6 @@ class TTestStats(BaseStats): Keeps statistics of two series of data and the probability both series have the same distribution. """ - # TODO # Generic Change Point List diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 5d0b674a..9f119128 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -45,10 +45,9 @@ .pivot() < - - > .pivot() """ -from collections import OrderedDict -from dataclasses import dataclass, fields +from dataclasses import dataclass, fields, replace from datetime import datetime, timezone -from typing import Any, Dict, Generic, List, Optional, Sequence, SupportsFloat, TypeVar +from typing import Dict, Generic, List, Optional, Sequence, SupportsFloat, TypeVar import numpy as np from numpy.typing import NDArray @@ -73,31 +72,34 @@ class BaseStats: std_1: float std_2: float - def __init__(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat], pvalue=None) -> Any: + @staticmethod + def calculate(left: Sequence[SupportsFloat], right: Sequence[SupportsFloat], pvalue=None): """ - Basic statsistics about the left and right side, and the change between them. + Compute basic statistics for the left and right sides of a change point. - Calculate basic statistics about the left and right sides of a change point, such as mean - standard deviation. p-value depeds on the significance test used, so we cannot know or compute - it here, but if the caller knows p already, they can supply it as argument. + Returns a new BaseStats holding the mean and standard deviation of each side. The + p-value depends on the significance test used, so we cannot compute it here; if the + caller already knows it they can pass it in, otherwise it defaults to 1.0. """ - self.calculate_base_stats(left, right, pvalue) - - def calculate_base_stats(self, left, right, pvalue=None): - if pvalue is not None and pvalue >= 0.0 and pvalue <= 1.0: - self.pvalue = pvalue - else: - self.pvalue = 1.0 - if len(left) == 0 or len(right) == 0: raise ValueError - self.mean_1 = np.mean(left) - self.mean_2 = np.mean(right) - self.std_1 = np.std(left) if len(left) >= 2 else 0.0 - self.std_2 = np.std(right) if len(right) >= 2 else 0.0 + if pvalue is not None and 0.0 <= pvalue <= 1.0: + p = pvalue + else: + p = 1.0 + + return BaseStats( + pvalue=p, + mean_1=np.mean(left), + mean_2=np.mean(right), + std_1=np.std(left) if len(left) >= 2 else 0.0, + std_2=np.std(right) if len(right) >= 2 else 0.0, + ) - return self + def copy(self): + """Return a copy of these statistics, preserving the concrete (sub)class.""" + return replace(self) def forward_rel_change(self, value_if_nan=0): """Relative change from left to right""" @@ -174,6 +176,16 @@ class ChangePoint(CandidateChangePoint, Generic[GenericStats]): # Which metric this change point belongs to. (This is redundant and for convenience.) metric: Optional[str] = None + def copy(self): + """ + Copy constructor. + + :return: A deep copy of self, recursively calls also stats.copy(). + """ + return ChangePoint( + index=self.index, qhat=self.qhat, stats=self.stats.copy(), metric=self.metric + ) + def __eq__(self, other): """Helpful to identify new Change Points during divisive algorithm""" return isinstance(other, self.__class__) and self.index == other.index @@ -283,17 +295,23 @@ def to_json(self, rounded=False): "changes": changes, } + def copy(self): + new_attributes = {k: v for k, v in self.attributes.items()} + new_changes = {metric: cp.copy() for metric, cp in self.changes.items()} + new_obj = ChangePointGroup(time=self.time, attributes=new_attributes, changes=new_changes) + return new_obj + def __getitem__(self, metric): return self.changes[metric] def metrics(self): return self.changes.keys() - def commit(self, idx: int): - return self.attribute_at(idx).get("commit") + def commit(self): + return self.attributes.get("commit") def datetime(self): - return datetime.fromtimestamp(self.time, timezone.UTC) + return datetime.fromtimestamp(self.time, timezone.utc) def select_metrics(self, m: list[str] | str): if not isinstance(m, list): @@ -326,50 +344,69 @@ class ChangePoints: Subclass ChangePointsByTime is this same class, but can be used if you explicitly want to mark the ordering. """ - def __init__(self, cps=None): - if isinstance(cps, dict) and not isinstance(cps, OrderedDict): + def __init__(self): + # The constructor only creates an empty container. To build one from + # existing data, use the explicit from_list() / from_dict() classmethods. + self.change_points = [] + + @classmethod + def from_list(cls, cps: list): + """Build from a list of ChangePointGroup objects, ordered by time.""" + if not isinstance(cps, list): raise TypeError( - "ChangePointsByTime doesn't accept a dict() as constructor input. Did you want ChangePointsByMetric()?" + f"from_list() takes a list of ChangePointGroup objects. Got {type(cps)}." ) - if isinstance(cps, OrderedDict): - for k, v in cps.items(): - if not isinstance(k, float): - raise TypeError( - "ChangePointsByTime with OrderedDict() as constructor input requires the keys to be float (timestamps)?" - ) - if not isinstance(v, ChangePointGroup): + for cpg in cps: + if not isinstance(cpg, ChangePointGroup): + raise TypeError( + f"from_list() takes a list of ChangePointGroup objects. Got {type(cpg)}." + ) + obj = cls() + for cpg in sorted(cps, key=lambda cpg: cpg.time): + obj.append(cpg) + return obj + + @classmethod + def from_dict(cls, cps: dict): + """ + Build a ChangePointsByMetric from a dict[str, list[ChangePointGroup]]. + + A dict is inherently keyed by metric, so from_dict() returns a + ChangePointsByMetric. (This is asymmetric with from_list(), which returns + whichever class you call it on.) ChangePointsByTime overrides this to reject + a dict outright. + """ + if not isinstance(cps, dict): + raise TypeError( + f"from_dict() takes a dict[str, list[ChangePointGroup]]. Got {type(cps)}." + ) + obj = ChangePointsByMetric() + for metric, cpglist in cps.items(): + for cpg in cpglist: + if not isinstance(cpg, ChangePointGroup): raise TypeError( - "ChangePointsByTime input must be an OrderedDict() of ChangePointGroup objects as values." + "from_dict() takes a dict[str, list[ChangePointGroup]]." ) - self.change_points = sorted(cps, key=lambda cpg: cpg.time) - return - if cps is None: - self.change_points = [] - return + # store each metric's groups sorted by timestamp + obj.change_points[metric] = sorted(cpglist, key=lambda cpg: cpg.time) + return obj - if isinstance(cps, ChangePointsByTime): - self.change_points = sorted(cps.change_points, key=lambda cpg: cpg.time) - return - if isinstance(cps, ChangePointsByMetric): - self.change_points = cps.pivot().change_points - return + def copy(self): + """ + Copy constructor. - if not isinstance(cps, list): - cps = [cps] - for obj in sorted(cps, key=lambda cpg: cpg.time): - if not isinstance(obj, ChangePointGroup): - t = type(obj) - raise TypeError( - f"ChangePoints() takes as argument one or more ChangePointGroup objects. Got {t}." - ) - self.change_points = cps + Returns a deep copy of this object. + """ + new_obj = ChangePointsByTime() + new_obj.change_points = [ + cpg.copy() for cpg in sorted(self.change_points, key=lambda cpg: cpg.time) + ] + return new_obj - def by_time(self) -> ChangePointsByTime: - cps = ChangePointsByTime() - cps.change_points = self.change_points - return cps + def by_time(self): + return self - def by_metric(self) -> ChangePointsByMetric: + def by_metric(self): return self.pivot() def append(self, cpg: ChangePointGroup): @@ -420,7 +457,7 @@ def __getitem__(self, n): def metrics(self) -> set: all_metrics = set() for row in self.change_points: - all_metrics.add(row.metrics()) + all_metrics.update(row.metrics()) return all_metrics def select_metrics(self, m: list[str] | str): @@ -441,7 +478,11 @@ def select_metrics(self, m: list[str] | str): def get_change_points_for_metric(self, m: str): single_metric = self.select_metrics(m) - return [list(cpg.changes.values())[0] for cpg in single_metric.change_points] + return [ + list(cpg.changes.values())[0] + for cpg in single_metric.change_points + if cpg.changes + ] def at_timestamp(self, t: float): for cpg in self.change_points: @@ -465,6 +506,14 @@ def pivot(self): class ChangePointsByTime(ChangePoints): + @classmethod + def from_dict(cls, cps: dict): + # A ChangePointsByTime is stored as a flat list of rows, not keyed by + # metric, so a dict has no meaningful representation here. + raise TypeError( + "ChangePointsByTime doesn't accept a dict. Did you want ChangePointsByMetric.from_dict()?" + ) + def pivot(self): """ Return the same object as ChangePointsByMetric. @@ -475,6 +524,7 @@ def pivot(self): assert isinstance(row, ChangePointGroup) # append() does the necessary shuffling into separate columns by_metric.append(row) + return by_metric class ChangePointsByMetric(ChangePoints): @@ -482,36 +532,28 @@ class ChangePointsByMetric(ChangePoints): Provides same interface as ChangePointsByTime, but internally stores with metric first. """ - def __init__(self, cps=None): - if isinstance(cps, ChangePointsByMetric): - self.change_points = cps.change_points - if isinstance(cps, ChangePointsByTime): - self.change_points = cps.pivot().change_points + def __init__(self): + # The constructor only creates an empty container. To build one from + # existing data, use the explicit from_list() / from_dict() classmethods. + # from_list() is inherited from ChangePoints; append() shuffles each + # group's metrics into their own column. + self.change_points = {} - if cps is None: - self.change_points = OrderedDict() - return - if isinstance(cps, list): - cpm = [] - for obj in sorted(cps, key=lambda cpg: cpg.time): - if not isinstance(obj, ChangePointGroup): - t = type(obj) - raise TypeError( - f"ChangePointsByMetric() takes as input a list of ChangePointGroup objects. Got {t}." - ) - cpm.append(obj) - - self.change_points = cpm - if isinstance(cps, dict): - # We actually don't need the ordering in this case, but we want the type to match the other class - self.change_points = OrderedDict() - for metric, cpglist in cps.items(): - for cpg in cpglist: - if not isinstance(cpg, ChangePointGroup): - raise TypeError( - "ChangePointsByMetric takes as constructor argument a dict of ChangePointGroup objects: dict[str, list[ChangePointGroup]]" - ) - self.change_points[metric] = sorted(cpglist, key=lambda cpg: cpg.time) + # from_dict() is inherited from ChangePoints: it already builds and returns a + # ChangePointsByMetric, which is exactly what we want here. + + def copy(self): + """ + Copy constructor. + + Returns a deep copy of this object. + """ + new_obj = ChangePointsByMetric() + new_obj.change_points = { + metric: [cpg.copy() for cpg in cpglist] + for metric, cpglist in self.change_points.items() + } + return new_obj def pivot(self): # Now we pivot (metric,time) to (time,metric) so that we return ChangePoints() objects @@ -542,6 +584,12 @@ def extend(self, cps): raise TypeError(errmsg) self.append(obj) + def by_time(self): + return self.pivot() + + def by_metric(self): + return self + def items(self): return self.change_points.items() @@ -615,7 +663,7 @@ def __init__(self, max_pvalue: float): def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat], p: float = None) -> GenericStats: if len(left) == 0 or len(right) == 0: raise ValueError - return BaseStats(left, right, p) + return BaseStats.calculate(left, right, p) def get_sides( self, candidate: CandidateChangePoint, series: Sequence[SupportsFloat], intervals: List[slice] diff --git a/otava/change_point_divisive/significance_test.py b/otava/change_point_divisive/significance_test.py index 29087a3f..94a1a8dd 100644 --- a/otava/change_point_divisive/significance_test.py +++ b/otava/change_point_divisive/significance_test.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -from dataclasses import dataclass +from dataclasses import dataclass, replace from typing import List, Optional, Type import numpy as np @@ -37,6 +37,10 @@ class PermutationStats(BaseStats): extreme_qhat_perm: int n_perm: int + def copy(self): + # replace() preserves the subclass; deep-copy the array so the copy is independent + return replace(self, permuted_qhats=self.permuted_qhats.copy()) + class PermutationsSignificanceTester(SignificanceTester): def __init__(self, max_pvalue: float, permutations: int, calculator: Type[Calculator], seed: Optional[int]): diff --git a/otava/series.py b/otava/series.py index 7f96ebce..522dc9af 100644 --- a/otava/series.py +++ b/otava/series.py @@ -210,7 +210,7 @@ def __compute_change_points( def __group_change_points_by_time( series: Series, change_points: ChangePoints ) -> ChangePointsByTime: - return ChangePointsByTime(change_points) + return change_points.by_time() def get_stable_range(self, metric: str, index: int) -> (int, int): """ @@ -320,8 +320,8 @@ def append(self, time, new_data, attributes): # TODO: Remove this. It should not be a requirement that metrics have the same history. fill_missing(self.__series.data[metric]) - r = ChangePointsByMetric(result) - w = ChangePointsByMetric(weak_change_points) + r = ChangePointsByMetric.from_dict(result) + w = ChangePointsByMetric.from_dict(weak_change_points) # print("#", self.change_points.change_points) # print("¤", r.change_points) # print("%", w.change_points) diff --git a/tests/change_point_classes_test.py b/tests/change_point_classes_test.py new file mode 100644 index 00000000..10e096b9 --- /dev/null +++ b/tests/change_point_classes_test.py @@ -0,0 +1,515 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Unit tests for the unified ChangePoint class hierarchy in +otava.change_point_divisive.base: + + BaseStats (+ TTestStats / PermutationStats) + CandidateChangePoint / ChangePoint / ChangePointSerializer + ChangePointGroup + ChangePoints / ChangePointsByTime / ChangePointsByMetric + SignificanceTester.calculate helpers (compare / get_sides / get_intervals) +""" + +import numpy as np +import pytest + +from otava.change_point_divisive.base import ( + BaseStats, + CandidateChangePoint, + ChangePoint, + ChangePointGroup, + ChangePoints, + ChangePointsByMetric, + ChangePointsByTime, + ChangePointSerializer, + SignificanceTester, +) + + +# --------------------------------------------------------------------------- # +# Helpers +# --------------------------------------------------------------------------- # +def make_stats(left=(1.0, 1.0), right=(5.0, 5.0), pvalue=0.01): + return BaseStats.calculate(list(left), list(right), pvalue) + + +def make_cp(metric="m", index=3, left=(1.0, 1.0), right=(5.0, 5.0), pvalue=0.01): + return ChangePoint( + index=index, qhat=1.0, stats=make_stats(left, right, pvalue), metric=metric + ) + + +def make_group(time, metric="m", index=3, commit="sha"): + return ChangePointGroup( + time=time, attributes={"commit": commit}, changes={metric: make_cp(metric, index)} + ) + + +# --------------------------------------------------------------------------- # +# BaseStats +# --------------------------------------------------------------------------- # +def test_basestats_calculate_means_and_std(): + s = BaseStats.calculate([10.0, 10.0, 10.0], [20.0, 20.0, 20.0], 0.02) + assert s.mean_1 == 10.0 + assert s.mean_2 == 20.0 + assert s.std_1 == 0.0 + assert s.std_2 == 0.0 + assert s.pvalue == 0.02 + # convenience getters + assert s.mean_before() == 10.0 + assert s.mean_after() == 20.0 + assert s.stddev_before() == 0.0 + assert s.stddev_after() == 0.0 + + +def test_basestats_single_element_side_has_zero_std(): + s = BaseStats.calculate([5.0], [7.0, 9.0]) + assert s.std_1 == 0.0 + assert s.std_2 > 0.0 + + +def test_basestats_pvalue_defaults_to_one(): + assert BaseStats.calculate([1.0, 1.0], [2.0, 2.0]).pvalue == 1.0 + # out-of-range values are ignored and fall back to 1.0 + assert BaseStats.calculate([1.0, 1.0], [2.0, 2.0], 5.0).pvalue == 1.0 + assert BaseStats.calculate([1.0, 1.0], [2.0, 2.0], -1.0).pvalue == 1.0 + + +def test_basestats_empty_side_raises(): + with pytest.raises(ValueError): + BaseStats.calculate([], [1.0, 2.0]) + with pytest.raises(ValueError): + BaseStats.calculate([1.0, 2.0], []) + + +def test_basestats_relative_change_and_magnitude(): + s = BaseStats.calculate([10.0, 10.0], [20.0, 20.0], 0.01) + assert s.forward_rel_change() == 1.0 + assert s.backward_rel_change() == -0.5 + assert s.forward_change_percent() == 100.0 + assert s.backward_change_percent() == -50.0 + assert s.change_magnitude() == 1.0 + + +def test_basestats_zero_mean_guard(): + s = BaseStats.calculate([0.0, 0.0], [1.0, 1.0]) + assert s.forward_rel_change() == 0 + assert s.forward_rel_change(value_if_nan=-1) == -1 + s2 = BaseStats.calculate([1.0, 1.0], [0.0, 0.0]) + assert s2.backward_rel_change() == 0 + + +def test_basestats_to_json_keys(): + s = BaseStats.calculate([10.0, 10.0], [20.0, 20.0], 0.02) + j = s.to_json() + assert set(j.keys()) == { + "forward_change_percent", + "magnitude", + "mean_before", + "stddev_before", + "mean_after", + "stddev_after", + "pvalue", + } + # values are rendered as strings + assert all(isinstance(v, str) for v in j.values()) + + +def test_basestats_copy_is_independent_and_keeps_class(): + s = make_stats() + c = s.copy() + assert isinstance(c, BaseStats) + assert c is not s + assert (c.mean_1, c.mean_2, c.pvalue) == (s.mean_1, s.mean_2, s.pvalue) + c.mean_1 = 999.0 + assert s.mean_1 != 999.0 + + +# --------------------------------------------------------------------------- # +# CandidateChangePoint / ChangePoint / ChangePointSerializer +# --------------------------------------------------------------------------- # +def test_changepoint_from_and_to_candidate(): + candidate = CandidateChangePoint(index=7, qhat=2.5) + stats = make_stats() + cp = ChangePoint.from_candidate(candidate, stats) + assert cp.index == 7 + assert cp.qhat == 2.5 + assert cp.stats is stats + + back = cp.to_candidate() + assert isinstance(back, CandidateChangePoint) + assert back.index == 7 + assert back.qhat == 2.5 + + +def test_changepoint_equality_is_by_index(): + a = make_cp(index=3) + b = make_cp(index=3, left=(2.0, 2.0)) # different stats, same index + c = make_cp(index=4) + assert a == b + assert a != c + + +def test_changepoint_metric_defaults_to_none(): + cp = ChangePoint(index=1, qhat=0.0, stats=make_stats()) + assert cp.metric is None + + +def test_changepoint_copy_is_deep(): + cp = make_cp() + clone = cp.copy() + assert clone is not None, "copy() must return the new object" + assert clone is not cp + assert clone.stats is not cp.stats + assert clone.index == cp.index + assert clone.qhat == cp.qhat + assert clone.metric == cp.metric + clone.stats.mean_1 = -1.0 + assert cp.stats.mean_1 != -1.0 + + +def test_changepoint_serializer_rounded_json(): + cp = make_cp(metric="m", index=3, left=(10.0, 10.0), right=(20.0, 20.0), pvalue=0.02) + j = ChangePointSerializer(cp).to_json(rounded=True) + assert j["metric"] == "m" + assert j["index"] == 3 + assert j["forward_change_percent"] == "100" + assert all(isinstance(j[k], str) for k in ("magnitude", "mean_before", "pvalue")) + + +def test_changepoint_serializer_unrounded_json(): + cp = make_cp(metric="m", index=3, left=(10.0, 10.0), right=(20.0, 20.0), pvalue=0.02) + j = ChangePointSerializer(cp).to_json(rounded=False) + assert j["index"] == 3 + assert j["forward_change_percent"] == 100.0 + assert j["mean_before"] == 10.0 + assert j["mean_after"] == 20.0 + assert j["pvalue"] == 0.02 + + +def test_changepoint_to_json_delegates_to_serializer(): + cp = make_cp(metric="m", index=3, left=(10.0, 10.0), right=(20.0, 20.0), pvalue=0.02) + assert cp.to_json(rounded=False) == ChangePointSerializer(cp).to_json(rounded=False) + + +# --------------------------------------------------------------------------- # +# ChangePointGroup +# --------------------------------------------------------------------------- # +def test_group_getitem_metrics_and_iter(): + g = ChangePointGroup( + time=1.0, + attributes={"commit": "abc"}, + changes={"m1": make_cp("m1"), "m2": make_cp("m2")}, + ) + assert g["m1"].metric == "m1" + assert set(g.metrics()) == {"m1", "m2"} + assert {cp.metric for cp in g} == {"m1", "m2"} + + +def test_group_select_metrics(): + g = ChangePointGroup( + time=1.0, + attributes={"commit": "abc"}, + changes={"m1": make_cp("m1"), "m2": make_cp("m2")}, + ) + one = g.select_metrics("m1") + assert set(one.metrics()) == {"m1"} + # accepts a list too + assert set(g.select_metrics(["m1", "m2"]).metrics()) == {"m1", "m2"} + # the original is untouched + assert set(g.metrics()) == {"m1", "m2"} + + +def test_group_set_adds_metric(): + g = make_group(1.0, metric="m1") + g.set("m2", make_cp("m2")) + assert set(g.metrics()) == {"m1", "m2"} + + +def test_group_commit_and_datetime(): + g = ChangePointGroup(time=0.0, attributes={"commit": "deadbeef"}, changes={"m": make_cp()}) + assert g.commit() == "deadbeef" + # 0.0 epoch seconds -> 1970, in UTC + assert g.datetime().year == 1970 + + +def test_group_to_json(): + g = make_group(1.0, metric="m", commit="abc") + j = g.to_json() + assert j["time"] == 1.0 + assert j["attributes"] == {"commit": "abc"} + assert isinstance(j["changes"], list) and len(j["changes"]) == 1 + assert j["changes"][0]["metric"] == "m" + + +def test_group_copy_is_deep(): + g = make_group(1.0, metric="m", commit="abc") + clone = g.copy() + assert isinstance(clone.attributes, dict) + assert isinstance(clone.changes, dict) + assert clone.attributes == {"commit": "abc"} + assert set(clone.metrics()) == {"m"} + # deep: mutating the clone's change does not touch the original + clone.changes["m"].stats.mean_1 = -1.0 + assert g.changes["m"].stats.mean_1 != -1.0 + + +# --------------------------------------------------------------------------- # +# ChangePointsByTime +# --------------------------------------------------------------------------- # +def test_bytime_append_keeps_time_order(): + c = ChangePointsByTime() + c.append(make_group(1.0, metric="a")) + c.append(make_group(2.0, metric="b")) + assert [g.time for g in c] == [1.0, 2.0] + assert len(c) == 2 + + +def test_bytime_append_same_time_merges_metrics(): + c = ChangePointsByTime() + c.append(make_group(1.0, metric="a")) + c.append(make_group(1.0, metric="b")) + assert len(c) == 1 + assert set(c[0].metrics()) == {"a", "b"} + + +def test_bytime_append_duplicate_metric_same_time_raises(): + c = ChangePointsByTime() + c.append(make_group(1.0, metric="a")) + with pytest.raises(KeyError): + c.append(make_group(1.0, metric="a")) + + +def test_bytime_append_decreasing_time_raises(): + c = ChangePointsByTime() + c.append(make_group(2.0, metric="a")) + with pytest.raises(ValueError): + c.append(make_group(1.0, metric="b")) + + +def test_bytime_constructor_sorts_and_validates(): + groups = [make_group(3.0, "a"), make_group(1.0, "b"), make_group(2.0, "c")] + c = ChangePointsByTime.from_list(groups) + assert [g.time for g in c] == [1.0, 2.0, 3.0] + # a single group must now be wrapped in a list (no convenience unwrapping) + with pytest.raises(TypeError): + ChangePointsByTime.from_list(make_group(1.0)) + + +def test_bytime_constructor_rejects_dict_and_non_group(): + with pytest.raises(TypeError): + ChangePointsByTime.from_dict({"m": []}) + with pytest.raises(TypeError): + ChangePointsByTime.from_list([object()]) + + +def test_bytime_extend(): + c = ChangePointsByTime() + c.extend([make_group(1.0, "a"), make_group(2.0, "b")]) + assert [g.time for g in c] == [1.0, 2.0] + with pytest.raises(TypeError): + c.extend("not a list") + + +def test_bytime_metrics_union(): + c = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + assert c.metrics() == {"a", "b"} + + +def test_bytime_at_timestamp_exact_and_tolerant(): + c = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + assert c.at_timestamp(2.0).time == 2.0 + # within tolerance + assert c.at_timestamp(2.00005).time == 2.0 + with pytest.raises(LookupError): + c.at_timestamp(99.0) + + +def test_bytime_at_commit(): + c = ChangePointsByTime.from_list( + [make_group(1.0, "a", commit="c1"), make_group(2.0, "b", commit="c2")] + ) + assert c.at_commit("c2").time == 2.0 + with pytest.raises(LookupError): + c.at_commit("nope") + + +def test_bytime_get_change_points_for_metric_sparse(): + # 'a' appears at t=1 and t=3, 'b' only at t=2 -> sparse columns + c = ChangePointsByTime.from_list( + [make_group(1.0, "a", index=1), make_group(2.0, "b", index=2), make_group(3.0, "a", index=3)] + ) + a_points = c.get_change_points_for_metric("a") + assert [cp.index for cp in a_points] == [1, 3] + assert [cp.index for cp in c.get_change_points_for_metric("b")] == [2] + + +def test_bytime_pivot_and_items(): + c = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + pivoted = c.pivot() + assert isinstance(pivoted, ChangePointsByMetric) + assert pivoted.metrics() == {"a", "b"} + assert {k for k, _ in c.items()} == {"a", "b"} + + +def test_bytime_copy_is_deep(): + c = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + clone = c.copy() + assert isinstance(clone, ChangePointsByTime) + assert len(clone) == 2 + clone[0].changes["a"].stats.mean_1 = -1.0 + assert c[0].changes["a"].stats.mean_1 != -1.0 + + +# --------------------------------------------------------------------------- # +# ChangePointsByMetric +# --------------------------------------------------------------------------- # +def test_bymetric_dict_constructor_sorts_by_time(): + g1, g2 = make_group(1.0, "m"), make_group(2.0, "m") + bm = ChangePointsByMetric.from_dict({"m": [g2, g1]}) # unsorted input + assert isinstance(bm, ChangePointsByMetric) + assert [g.time for g in bm.change_points["m"]] == [1.0, 2.0] + + +def test_bymetric_dict_constructor_rejects_non_group(): + with pytest.raises(TypeError): + ChangePointsByMetric.from_dict({"m": [object()]}) + + +def test_from_dict_is_metric_keyed_and_returns_by_metric(): + # A dict is inherently keyed by metric, so from_dict() always builds a + # ChangePointsByMetric -- even when called on the base ChangePoints class. + result = ChangePoints.from_dict({"m": [make_group(1.0, "m")]}) + assert isinstance(result, ChangePointsByMetric) + assert result.metrics() == {"m"} + + +def test_bymetric_list_constructor(): + bm = ChangePointsByMetric.from_list([make_group(1.0, "a"), make_group(2.0, "a"), make_group(2.0, "b")]) + # from_list() returns the class it is called on (asymmetric with from_dict) + assert isinstance(bm, ChangePointsByMetric) + assert bm.metrics() == {"a", "b"} + assert [cp.index for cp in bm.get_change_points_for_metric("a")] == [3, 3] + + +def test_bymetric_append_and_len(): + bm = ChangePointsByMetric() + bm.append(make_group(1.0, "a")) + bm.append(make_group(2.0, "a")) + bm.append(make_group(1.0, "b")) + assert bm.metrics() == {"a", "b"} + # len == longest column + assert len(bm) == 2 + + +def test_bymetric_select_metrics(): + bm = ChangePointsByMetric.from_list([make_group(1.0, "a"), make_group(1.0, "b")]) + only_a = bm.select_metrics("a") + assert only_a.metrics() == {"a"} + + +def test_bymetric_items_and_iter(): + bm = ChangePointsByMetric.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + assert {k for k, _ in bm.items()} == {"a", "b"} + # iterating yields ChangePointGroups (via pivot to time order) + times = [g.time for g in bm] + assert times == sorted(times) + + +def test_bymetric_by_time_roundtrip(): + by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + bm = by_time.pivot() + again = bm.by_time() + assert isinstance(again, ChangePointsByTime) + assert [g.time for g in again] == [1.0, 2.0] + + +def test_bymetric_at_timestamp_and_at_commit(): + bm = ChangePointsByMetric.from_list( + [make_group(1.0, "a", commit="c1"), make_group(2.0, "b", commit="c2")] + ) + assert bm.at_timestamp(2.0).time == 2.0 + assert bm.at_commit("c1").time == 1.0 + + +def test_bymetric_copy_is_deep(): + bm = ChangePointsByMetric.from_list([make_group(1.0, "a"), make_group(2.0, "a")]) + clone = bm.copy() + assert isinstance(clone, ChangePointsByMetric) + assert clone.metrics() == {"a"} + clone.change_points["a"][0].changes["a"].stats.mean_1 = -1.0 + assert bm.change_points["a"][0].changes["a"].stats.mean_1 != -1.0 + + +# --------------------------------------------------------------------------- # +# SignificanceTester helpers +# --------------------------------------------------------------------------- # +def test_tester_compare_returns_basestats(): + tester = SignificanceTester(0.05) + stats = tester.compare([1.0, 1.0], [5.0, 5.0], 0.01) + assert isinstance(stats, BaseStats) + assert stats.mean_1 == 1.0 + assert stats.mean_2 == 5.0 + assert stats.pvalue == 0.01 + + +def test_tester_compare_empty_raises(): + tester = SignificanceTester(0.05) + with pytest.raises(ValueError): + tester.compare([], [1.0]) + + +def test_tester_get_intervals(): + tester = SignificanceTester(0.05) + intervals = tester.get_intervals([make_cp(index=3)]) + assert intervals == [slice(0, 3), slice(3, None)] + + +def test_tester_get_sides_split_step(): + tester = SignificanceTester(0.05) + series = np.array([1.0, 1.0, 1.0, 5.0, 5.0, 5.0]) + intervals = tester.get_intervals([make_cp(index=3)]) + left, right = tester.get_sides(CandidateChangePoint(index=3, qhat=1.0), series, intervals) + assert list(left) == [1.0, 1.0, 1.0] + assert list(right) == [5.0, 5.0, 5.0] + + +def test_tester_get_sides_merge_step(): + tester = SignificanceTester(0.05) + series = np.arange(9, dtype=float) + intervals = [slice(0, 3), slice(3, 6), slice(6, None)] + # candidate index matches the stop of the first interval -> merge step + left, right = tester.get_sides(CandidateChangePoint(index=3, qhat=1.0), series, intervals) + assert list(left) == [0.0, 1.0, 2.0] + assert list(right) == [3.0, 4.0, 5.0] + + +def test_tester_is_significant(): + tester = SignificanceTester(0.05) + assert tester.is_significant(make_cp(pvalue=0.01)) is True + assert tester.is_significant(make_cp(pvalue=0.5)) is False + + +# --------------------------------------------------------------------------- # +# Base ChangePoints behaviours shared by both representations +# --------------------------------------------------------------------------- # +def test_base_changepoints_empty(): + c = ChangePoints() + assert len(c) == 0 + assert list(c) == [] From ec98196ec68a508f794ad7e96a0b2a12da8c5462 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 8 Jun 2026 05:45:35 +0300 Subject: [PATCH 11/27] Make ChangePoints._change_points private by adding _ --- otava/change_point_divisive/base.py | 74 ++++++++++++++++------------- otava/series.py | 7 +-- tests/change_point_classes_test.py | 14 ++++-- tests/series_test.py | 12 ++--- 4 files changed, 59 insertions(+), 48 deletions(-) diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 9f119128..1feb2e73 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -41,7 +41,7 @@ | ^ ChangePointsByTime `ChangePointsByMetric - .change_points: list(ChangePointGroup) .change_points: dict[metric, list(ChangePointGroup)] + ._change_points: list(ChangePointGroup) ._change_points: dict[metric, list(ChangePointGroup)] .pivot() < - - > .pivot() """ @@ -347,7 +347,7 @@ class ChangePoints: def __init__(self): # The constructor only creates an empty container. To build one from # existing data, use the explicit from_list() / from_dict() classmethods. - self.change_points = [] + self._change_points = [] @classmethod def from_list(cls, cps: list): @@ -388,7 +388,7 @@ def from_dict(cls, cps: dict): "from_dict() takes a dict[str, list[ChangePointGroup]]." ) # store each metric's groups sorted by timestamp - obj.change_points[metric] = sorted(cpglist, key=lambda cpg: cpg.time) + obj._change_points[metric] = sorted(cpglist, key=lambda cpg: cpg.time) return obj def copy(self): @@ -398,8 +398,8 @@ def copy(self): Returns a deep copy of this object. """ new_obj = ChangePointsByTime() - new_obj.change_points = [ - cpg.copy() for cpg in sorted(self.change_points, key=lambda cpg: cpg.time) + new_obj._change_points = [ + cpg.copy() for cpg in sorted(self._change_points, key=lambda cpg: cpg.time) ] return new_obj @@ -413,16 +413,16 @@ def append(self, cpg: ChangePointGroup): if not isinstance(cpg, ChangePointGroup): raise TypeError("ChangePoints.append() takes as argument one ChangePointGroup.") - if (not self.change_points) or cpg.time > self.change_points[-1].time: - self.change_points.append(cpg) - elif self.change_points and cpg.time == self.change_points[-1].time: + if (not self._change_points) or cpg.time > self._change_points[-1].time: + self._change_points.append(cpg) + elif self._change_points and cpg.time == self._change_points[-1].time: for metric, cp in cpg.changes.items(): - if metric in self.change_points[-1].changes: + if metric in self._change_points[-1].changes: raise KeyError("Duplicate keys. Shouldn't happen.") - self.change_points[-1].changes[metric] = cp + self._change_points[-1].changes[metric] = cp else: # TODO: logging - # print(self.change_points) + # print(self._change_points) # print(cpg) raise ValueError( "ChangePoints.append() can only be used such that time is monotonically increasing" @@ -435,8 +435,8 @@ def extend(self, cps): for obj in cps: if not isinstance(obj, ChangePointGroup): raise TypeError(errmsg) - if (not self.change_points) or obj.time > self.change_points[-1].time: - self.change_points.append(obj) + if (not self._change_points) or obj.time > self._change_points[-1].time: + self._change_points.append(obj) else: raise ValueError( "ChangePoints.extend() can only be used such that time is monotonically increasing" @@ -446,17 +446,17 @@ def items(self): return self.pivot().items() def __iter__(self): - return iter(self.change_points) + return iter(self._change_points) def __len__(self): - return len(self.change_points) + return len(self._change_points) def __getitem__(self, n): - return self.change_points[n] + return self._change_points[n] def metrics(self) -> set: all_metrics = set() - for row in self.change_points: + for row in self._change_points: all_metrics.update(row.metrics()) return all_metrics @@ -472,7 +472,7 @@ def select_metrics(self, m: list[str] | str): Use ChangePointsByMetric if you need this to be fast. """ filtered = ChangePoints() - for cpg in self.change_points: + for cpg in self._change_points: filtered.append(cpg.select_metrics(m)) return filtered @@ -480,12 +480,12 @@ def get_change_points_for_metric(self, m: str): single_metric = self.select_metrics(m) return [ list(cpg.changes.values())[0] - for cpg in single_metric.change_points + for cpg in single_metric._change_points if cpg.changes ] def at_timestamp(self, t: float): - for cpg in self.change_points: + for cpg in self._change_points: if cpg.time == t: return cpg if abs(cpg.time - t) < 0.0001: @@ -520,7 +520,7 @@ def pivot(self): """ by_metric = ChangePointsByMetric() - for row in sorted(self.change_points, key=lambda cpg: cpg.time): + for row in sorted(self._change_points, key=lambda cpg: cpg.time): assert isinstance(row, ChangePointGroup) # append() does the necessary shuffling into separate columns by_metric.append(row) @@ -537,7 +537,7 @@ def __init__(self): # existing data, use the explicit from_list() / from_dict() classmethods. # from_list() is inherited from ChangePoints; append() shuffles each # group's metrics into their own column. - self.change_points = {} + self._change_points = {} # from_dict() is inherited from ChangePoints: it already builds and returns a # ChangePointsByMetric, which is exactly what we want here. @@ -549,16 +549,16 @@ def copy(self): Returns a deep copy of this object. """ new_obj = ChangePointsByMetric() - new_obj.change_points = { + new_obj._change_points = { metric: [cpg.copy() for cpg in cpglist] - for metric, cpglist in self.change_points.items() + for metric, cpglist in self._change_points.items() } return new_obj def pivot(self): # Now we pivot (metric,time) to (time,metric) so that we return ChangePoints() objects intermediate = [] - for metric, points in self.change_points.items(): + for metric, points in self._change_points.items(): for cpg in sorted(points, key=lambda cpg: cpg.time): assert isinstance(cpg, ChangePointGroup) intermediate.append(cpg) @@ -571,9 +571,9 @@ def append(self, cpg: ChangePointGroup): if not isinstance(cpg, ChangePointGroup): raise TypeError("ChangePoints.append() takes as argument one ChangePointGroup.") for metric in cpg.metrics(): - if metric not in self.change_points: - self.change_points[metric] = [] - self.change_points[metric].append(cpg.select_metrics(metric)) + if metric not in self._change_points: + self._change_points[metric] = [] + self._change_points[metric].append(cpg.select_metrics(metric)) def extend(self, cps): errmsg = "ChangePoints.extend() takes as argument a list of ChangePointGroup objects." @@ -591,18 +591,18 @@ def by_metric(self): return self def items(self): - return self.change_points.items() + return self._change_points.items() def __iter__(self): return self.pivot().__iter__() def __len__(self): - return max([len(cpg) for metric, cpg in self.change_points.items()]) + return max([len(cpg) for metric, cpg in self._change_points.items()]) def __getitem__(self, n): if not isinstance(n, int): raise KeyError("n must be integer index") - cpgrow = [cpg[n] for metric, cpg in self.change_points.items() if len(cpg) > n] + cpgrow = [cpg[n] for metric, cpg in self._change_points.items() if len(cpg) > n] if not cpgrow: KeyError(f"Nice try {n}. This ChangePoints object only has {len(self)} items.") cpg = None @@ -616,8 +616,14 @@ def __getitem__(self, n): return cpg + def __setitem__(self, metric: str, cpglist: ChangePointGroup): + self._change_points[metric] = cpglist + + def __in__(self, metric): + return metric in self._change_points + def metrics(self): - return set(self.change_points.keys()) + return set(self._change_points.keys()) def select_metrics(self, m: list[str] | str): """ @@ -630,13 +636,13 @@ def select_metrics(self, m: list[str] | str): filtered = ChangePointsByMetric() for metric in m: - filtered.change_points[metric] = self.change_points[metric] + filtered._change_points[metric] = self._change_points[metric] return filtered def get_change_points_for_metric(self, m: str): single_metric = self.select_metrics(m) metric_change_points = [] - for metric, cpg in single_metric.change_points.items(): + for metric, cpg in single_metric._change_points.items(): for c in cpg: metric_change_points.append(c.changes[metric]) return metric_change_points diff --git a/otava/series.py b/otava/series.py index 522dc9af..15112287 100644 --- a/otava/series.py +++ b/otava/series.py @@ -322,12 +322,9 @@ def append(self, time, new_data, attributes): r = ChangePointsByMetric.from_dict(result) w = ChangePointsByMetric.from_dict(weak_change_points) - # print("#", self.change_points.change_points) - # print("¤", r.change_points) - # print("%", w.change_points) # r has a subset of all metrics, so can't just set change_points to r - for metric, cpglist in r.change_points.items(): - self.change_points.change_points[metric] = cpglist + for metric, cpglist in r.items(): + self.change_points[metric] = cpglist return r, w def test_name(self) -> str: diff --git a/tests/change_point_classes_test.py b/tests/change_point_classes_test.py index 10e096b9..82a52157 100644 --- a/tests/change_point_classes_test.py +++ b/tests/change_point_classes_test.py @@ -384,7 +384,7 @@ def test_bymetric_dict_constructor_sorts_by_time(): g1, g2 = make_group(1.0, "m"), make_group(2.0, "m") bm = ChangePointsByMetric.from_dict({"m": [g2, g1]}) # unsorted input assert isinstance(bm, ChangePointsByMetric) - assert [g.time for g in bm.change_points["m"]] == [1.0, 2.0] + assert [g.time for g in bm._change_points["m"]] == [1.0, 2.0] def test_bymetric_dict_constructor_rejects_non_group(): @@ -453,8 +453,16 @@ def test_bymetric_copy_is_deep(): clone = bm.copy() assert isinstance(clone, ChangePointsByMetric) assert clone.metrics() == {"a"} - clone.change_points["a"][0].changes["a"].stats.mean_1 = -1.0 - assert bm.change_points["a"][0].changes["a"].stats.mean_1 != -1.0 + clone._change_points["a"][0].changes["a"].stats.mean_1 = -1.0 + assert bm._change_points["a"][0].changes["a"].stats.mean_1 != -1.0 + + +def test_bymetric__setitem_in(): + bm = ChangePointsByMetric.from_list([make_group(1.0, "a"), make_group(2.0, "a")]) + bm["x"] = make_group(99.9, metric="x", index=55) + assert "x" in bm._change_points + assert bm._change_points["x"].time == 99.9 + assert bm._change_points["x"].changes["x"].index == 55 # --------------------------------------------------------------------------- # diff --git a/tests/series_test.py b/tests/series_test.py index 51112384..9b3e812e 100644 --- a/tests/series_test.py +++ b/tests/series_test.py @@ -39,10 +39,10 @@ def test_change_point_detection(): cps = test.analyze().change_points_by_time assert len(cps) == 2 - assert cps.change_points[0].time == 4 - assert cps.change_points[0].changes["series2"].metric == "series2" - assert cps.change_points[1].time == 6 - assert cps.change_points[1].changes["series1"].metric == "series1" + assert cps._change_points[0].time == 4 + assert cps._change_points[0].changes["series2"].metric == "series2" + assert cps._change_points[1].time == 6 + assert cps._change_points[1].changes["series1"].metric == "series1" def test_change_point_detection_many(): @@ -94,7 +94,7 @@ def test_change_point_detection_many(): analyzed_series = test.analyze(options) assert len(list(analyzed_series.change_points)) == 3 cps_by_time = analyzed_series.change_points_by_time - assert len(cps_by_time.change_points) == 3 + assert len(cps_by_time._change_points) == 3 assert analyzed_series.change_points[0].time == 5 assert "series3" in analyzed_series.change_points[0].changes @@ -116,7 +116,7 @@ def test_change_point_min_magnitude(): options.min_magnitude = 0.2 cps = test.analyze(options).change_points_by_time assert len(cps) == 1 - assert cps.change_points[0].time == 6 + assert cps._change_points[0].time == 6 assert "series1" in cps[0].changes for change_point in cps: From 086175d3c9e1b870fe437b521056a7cb6ae21cf6 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 8 Jun 2026 07:00:29 +0300 Subject: [PATCH 12/27] ChangePointsByMetric()[n] is not well defined, go via by_time() Other small fixes --- otava/change_point_divisive/base.py | 56 ++++++++++++++--------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 1feb2e73..f5382736 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -345,8 +345,10 @@ class ChangePoints: """ def __init__(self): - # The constructor only creates an empty container. To build one from - # existing data, use the explicit from_list() / from_dict() classmethods. + """ + The constructor only creates an empty container. To build one from + existing data, use the explicit from_list() / from_dict() classmethods. + """ self._change_points = [] @classmethod @@ -354,7 +356,7 @@ def from_list(cls, cps: list): """Build from a list of ChangePointGroup objects, ordered by time.""" if not isinstance(cps, list): raise TypeError( - f"from_list() takes a list of ChangePointGroup objects. Got {type(cps)}." + f"from_list() argument must be a list. Got {type(cps)}." ) for cpg in cps: if not isinstance(cpg, ChangePointGroup): @@ -387,6 +389,9 @@ def from_dict(cls, cps: dict): raise TypeError( "from_dict() takes a dict[str, list[ChangePointGroup]]." ) + for cp in cpg: + if cp.metric and cp.metric != metric: + raise ValueError(f"metric field is not internally consistent. {cp.metric} != {metric} at {cpg.time}") # store each metric's groups sorted by timestamp obj._change_points[metric] = sorted(cpglist, key=lambda cpg: cpg.time) return obj @@ -499,17 +504,16 @@ def at_commit(self, sha: str): raise LookupError(sha) def pivot(self): - """ - Return the same object as ChangePointsByMetric. - """ raise NotImplementedError() class ChangePointsByTime(ChangePoints): @classmethod def from_dict(cls, cps: dict): - # A ChangePointsByTime is stored as a flat list of rows, not keyed by - # metric, so a dict has no meaningful representation here. + """ + A ChangePointsByTime is stored as a flat list of rows, not keyed by + metric, so a dict has no meaningful representation here. + """ raise TypeError( "ChangePointsByTime doesn't accept a dict. Did you want ChangePointsByMetric.from_dict()?" ) @@ -533,10 +537,12 @@ class ChangePointsByMetric(ChangePoints): """ def __init__(self): - # The constructor only creates an empty container. To build one from - # existing data, use the explicit from_list() / from_dict() classmethods. - # from_list() is inherited from ChangePoints; append() shuffles each - # group's metrics into their own column. + """ + The constructor only creates an empty container. To build one from + existing data, use the explicit from_list() / from_dict() classmethods. + from_list() is inherited from ChangePoints; append() shuffles each + group's metrics into their own column. + """ self._change_points = {} # from_dict() is inherited from ChangePoints: it already builds and returns a @@ -556,7 +562,9 @@ def copy(self): return new_obj def pivot(self): - # Now we pivot (metric,time) to (time,metric) so that we return ChangePoints() objects + """ + Pivot (metric,time) to (time,metric) so that we return ChangePointsByTime() objects + """ intermediate = [] for metric, points in self._change_points.items(): for cpg in sorted(points, key=lambda cpg: cpg.time): @@ -600,21 +608,13 @@ def __len__(self): return max([len(cpg) for metric, cpg in self._change_points.items()]) def __getitem__(self, n): - if not isinstance(n, int): - raise KeyError("n must be integer index") - cpgrow = [cpg[n] for metric, cpg in self._change_points.items() if len(cpg) > n] - if not cpgrow: - KeyError(f"Nice try {n}. This ChangePoints object only has {len(self)} items.") - cpg = None - for c in cpgrow: - if cpg is None: - # First element - cpg = c - continue - for metric in cpg.metrics(): - cpg[metric] = c - - return cpg + """ + Returns the same ChangePointGroup as (ChangePointsByTime)[n] would. + + Hence this calls pivot() and is slow. + Alternatively please use .at_timestamp(), .at_commit() or .get_change_points_for_metric() instead. + """ + return self.by_time()[n] def __setitem__(self, metric: str, cpglist: ChangePointGroup): self._change_points[metric] = cpglist From 422aaf58afd40bbbb5fe5466b07eafa3c4f9cc53 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 8 Jun 2026 08:07:40 +0300 Subject: [PATCH 13/27] Visit all change points and columns, not just values()[0] --- otava/change_point_divisive/base.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index f5382736..5e002ce0 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -483,11 +483,13 @@ def select_metrics(self, m: list[str] | str): def get_change_points_for_metric(self, m: str): single_metric = self.select_metrics(m) - return [ - list(cpg.changes.values())[0] - for cpg in single_metric._change_points - if cpg.changes - ] + metric_change_points = [] + for cpg in single_metric._change_points: + for metric, cp in cpg.changes.items(): + if cp.metric and cp.metric != metric: + raise ValueError(f"metric field is not internally consistent. {cp.metric} != {metric} at {cpg.time}") + metric_change_points.append(cp) + return metric_change_points def at_timestamp(self, t: float): for cpg in self._change_points: @@ -642,9 +644,13 @@ def select_metrics(self, m: list[str] | str): def get_change_points_for_metric(self, m: str): single_metric = self.select_metrics(m) metric_change_points = [] - for metric, cpg in single_metric._change_points.items(): - for c in cpg: - metric_change_points.append(c.changes[metric]) + for metric1, cpglist in single_metric._change_points.items(): + for cpg in cpglist: + for metric2, cp in cpg.changes.items(): + if metric1 != metric2 or cp.metric and metric1!= cp.metric: + raise ValueError(f"metric field is not internally consistent. {metric1} != {cp.metric} at {cpg.time}") + metric_change_points.append(cp) + return metric_change_points def at_timestamp(self, t: float): From 3b03219332e75560f1d76242cb6fc87b3ada774d Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 8 Jun 2026 08:18:44 +0300 Subject: [PATCH 14/27] Add comment about why indexes will not be aligned or can have gaps --- otava/change_point_divisive/base.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 5e002ce0..2cb2418c 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -336,7 +336,10 @@ class ChangePoints: Typical usage of this would be to hold all the change points over a history of a single test, the test producing one or more metrics. Note that this is a sparse structure: It is NOT guaranteed that each row (each GhangePointGroup) has each metric. Similarly it is not guaranteed - that a given metric will hold the full sequence. + that a given metric will hold the full sequence. For example this could happen if a test configuration + adds more metrics to track over time. Then their indexing starts at different points in time and cannot ever + be the same. There could also be gaps, for example if some thread level failed, but others succeeded, + in the most recent results, then we can still use Otava on each history of results, each with their own index. Companion class ChangePointsByMetric is expected to provide functionally equivalent interface, but storing each series separately by metric, which is used in parts of the code base, in particular, what @@ -645,11 +648,11 @@ def get_change_points_for_metric(self, m: str): single_metric = self.select_metrics(m) metric_change_points = [] for metric1, cpglist in single_metric._change_points.items(): - for cpg in cpglist: - for metric2, cp in cpg.changes.items(): - if metric1 != metric2 or cp.metric and metric1!= cp.metric: - raise ValueError(f"metric field is not internally consistent. {metric1} != {cp.metric} at {cpg.time}") - metric_change_points.append(cp) + for cpg in cpglist: + for metric2, cp in cpg.changes.items(): + if metric1 != metric2 or cp.metric and metric1 != cp.metric: + raise ValueError(f"metric field is not internally consistent. {metric1} != {cp.metric} at {cpg.time}") + metric_change_points.append(cp) return metric_change_points From 6037ef989ece7f7175ed6647f4effeafe1aad3c7 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Tue, 16 Jun 2026 18:13:37 +0300 Subject: [PATCH 15/27] TTestStats: Add tstatistic and degrees_of_freedom Add a few t-test specific parameters to the TTestStats class --- otava/analysis.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/otava/analysis.py b/otava/analysis.py index cee0dcf6..f89745b1 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -39,9 +39,12 @@ @dataclass class TTestStats(BaseStats): """ - Keeps statistics of two series of data and the probability both series - have the same distribution. + Statistics related to the calculation of a two-sided Student's T-test. + + Note that p-value is already in BaseStats. """ + tstatistic: float = 0.0 + degrees_of_freedom: int = 0 # Generic Change Point List @@ -62,15 +65,24 @@ class TTestSignificanceTester(SignificanceTester): """ def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat]) -> TTestStats: + # defaults + p = 1.0 + t = 0.0 + base_stats = super().compare(left, right) - if len(left) + len(right) > 2: - (_, p) = ttest_ind_from_stats( + df = len(left) + len(right) - 2 + # While "degrees of freedom" is a parameter that is here coming out of the use of T-Test, + # this is actually the same requirement as can be expressed more intuitively as: + # We need at least 3 data points to find a statistically significant change point. + # This is because if we had only 2 points, they can be either equal or different, but if they + # are different, we have no context for how different they have to be to be statistically significant. + if df > 0: + (t, p) = ttest_ind_from_stats( base_stats.mean_1, base_stats.std_1, len(left), base_stats.mean_2, base_stats.std_2, len(right), alternative="two-sided" ) - else: - p = 1.0 - return TTestStats(pvalue=p, mean_1=base_stats.mean_1, mean_2=base_stats.mean_2, std_1=base_stats.std_1, std_2=base_stats.std_2) + + return TTestStats(pvalue=p, mean_1=base_stats.mean_1, mean_2=base_stats.mean_2, std_1=base_stats.std_1, std_2=base_stats.std_2, tstatistic=t, degrees_of_freedom=df) def change_point( self, From 84d6d413954101b1a71b6ea759829c21247c9912 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Tue, 16 Jun 2026 19:31:49 +0300 Subject: [PATCH 16/27] Add to_json() serializers to subclasses of BaseStats Note: No from_json(), as the higher level from_json() functions can easily just call the constructor directly. --- otava/analysis.py | 11 +++++++++++ otava/change_point_divisive/significance_test.py | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/otava/analysis.py b/otava/analysis.py index f89745b1..61c92481 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -46,6 +46,17 @@ class TTestStats(BaseStats): tstatistic: float = 0.0 degrees_of_freedom: int = 0 + def copy(self): + # replace() preserves the subclass; deep-copy the array so the copy is independent + return replace(self, permuted_qhats=self.permuted_qhats.copy()) + + def to_json(self): + obj = super().to_json() + obj["permuted_qhats"] = self.permuted_qhats.tolist() + obj["extreme_qhat_perm"] = self.extreme_qhat_perm + obj["n_perm"] = self.n_perm + return obj + # Generic Change Point List GenCPList = List[ChangePoint[GenericStats]] diff --git a/otava/change_point_divisive/significance_test.py b/otava/change_point_divisive/significance_test.py index 94a1a8dd..dcbddd42 100644 --- a/otava/change_point_divisive/significance_test.py +++ b/otava/change_point_divisive/significance_test.py @@ -41,6 +41,13 @@ def copy(self): # replace() preserves the subclass; deep-copy the array so the copy is independent return replace(self, permuted_qhats=self.permuted_qhats.copy()) + def to_json(self): + obj = super().to_json() + obj["permuted_qhats"] = self.permuted_qhats.tolist() + obj["extreme_qhat_perm"] = self.extreme_qhat_perm + obj["n_perm"] = self.n_perm + return obj + class PermutationsSignificanceTester(SignificanceTester): def __init__(self, max_pvalue: float, permutations: int, calculator: Type[Calculator], seed: Optional[int]): From 2be4bbf5cbdb8721a381b495d3806bda114c040e Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Tue, 16 Jun 2026 21:15:39 +0300 Subject: [PATCH 17/27] Self review, various fixes Added typing annotations and docstrings where I felt they were lacking. Cleaned up the serialization and insert to bigquery and postgres. Both of which continue to insert ChangePoint(s), even if they take a ChangePointGroup as arguments. This is because .time and .attributes are now both a property of the ChangePointGroup. --- otava/analysis.py | 3 +- otava/bigquery.py | 7 +-- otava/change_point_divisive/base.py | 67 ++++++++++++++++------------- otava/main.py | 24 +++++------ otava/series.py | 8 ++-- tests/change_point_divisive_test.py | 2 +- 6 files changed, 58 insertions(+), 53 deletions(-) diff --git a/otava/analysis.py b/otava/analysis.py index 61c92481..518d66c7 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -16,7 +16,7 @@ # under the License. import copy -from dataclasses import dataclass +from dataclasses import dataclass, replace from typing import List, Optional, Sequence, SupportsFloat, Tuple from scipy.stats import ttest_ind_from_stats @@ -92,7 +92,6 @@ def compare(self, left: Sequence[SupportsFloat], right: Sequence[SupportsFloat]) base_stats.mean_1, base_stats.std_1, len(left), base_stats.mean_2, base_stats.std_2, len(right), alternative="two-sided" ) - return TTestStats(pvalue=p, mean_1=base_stats.mean_1, mean_2=base_stats.mean_2, std_1=base_stats.std_1, std_2=base_stats.std_2, tstatistic=t, degrees_of_freedom=df) def change_point( diff --git a/otava/bigquery.py b/otava/bigquery.py index 34f329d2..729ffdc7 100644 --- a/otava/bigquery.py +++ b/otava/bigquery.py @@ -22,7 +22,7 @@ from google.cloud import bigquery from google.oauth2 import service_account -from otava.analysis import ChangePoint +from otava.change_point_divisive.base import ChangePointGroup from otava.test_config import BigQueryTestConfig @@ -87,9 +87,10 @@ def insert_change_point( test: BigQueryTestConfig, metric_name: str, attributes: Dict, - change_point: ChangePoint, + change_point_group: ChangePointGroup, ): - kwargs = {**attributes, **{test.time_column: datetime.utcfromtimestamp(change_point.time)}} + change_point = change_point_group[metric_name] + kwargs = {**attributes, **{test.time_column: datetime.utcfromtimestamp(change_point_group.time)}} update_stmt = test.update_stmt.format( metric=metric_name, forward_change_percent=change_point.forward_change_percent(), diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 2cb2418c..2e8f626c 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -277,8 +277,20 @@ def to_json(self, rounded=True): @dataclass class ChangePointGroup: - """A group of change points on multiple metrics, at the same time""" - + """ + A group of change points on multiple metrics, at the same point in time. + + If you think of ChangePoints as a 2D table, then ChangePointGroup are rows, and the metrics are the columns. + One ChangePointGroup has only the metrics where a ChangePoint was found at this time(stamp). + Note that there is no .index at this level. This is because each metric is an independent sequence of results, + and each such sequence may have started at different times in the past, and may also be missing observations + where others do have some. Because of this, each ChangePoint has its own .index in + ChangePointGroup.changes[metric_name].index + + :param time: The unix timestamp for the results that ChangePoint(s) were found at. Decimals are milliseconds. + :param attributes: The attributes of the test result at this timestamp. Commit and test metadata. + :param changes: For each metric that has a change point at this time(stamp), the ChangePoint object. + """ time: float attributes: Dict[str, str] # ChangePointGroup.changes.keys() stores the set of metrics that were used at this ChangePointGroup.time. @@ -334,17 +346,15 @@ class ChangePoints: A list of ChangePointGroup objects. Typical usage of this would be to hold all the change points over a history of a single test, - the test producing one or more metrics. Note that this is a sparse structure: It is NOT - guaranteed that each row (each GhangePointGroup) has each metric. Similarly it is not guaranteed - that a given metric will hold the full sequence. For example this could happen if a test configuration - adds more metrics to track over time. Then their indexing starts at different points in time and cannot ever - be the same. There could also be gaps, for example if some thread level failed, but others succeeded, - in the most recent results, then we can still use Otava on each history of results, each with their own index. + the test producing one or more metrics. Note that this is a sparse structure. It only + holds ChangePoint objects at the time and metric (row and column...) that one was found. + + Specifically: it is NOT guaranteed that each row (each GhangePointGroup) has each metric. Companion class ChangePointsByMetric is expected to provide functionally equivalent interface, but storing each series separately by metric, which is used in parts of the code base, in particular, what Series.analyze() returns. - Subclass ChangePointsByTime is this same class, but can be used if you explicitly want to mark the ordering. + Subclass ChangePointsByTime is this same class, but can be used if you explicitly want to mark the ordering used. """ def __init__(self): @@ -355,7 +365,7 @@ def __init__(self): self._change_points = [] @classmethod - def from_list(cls, cps: list): + def from_list(cls, cps: list[ChangePointGroup]): """Build from a list of ChangePointGroup objects, ordered by time.""" if not isinstance(cps, list): raise TypeError( @@ -436,7 +446,7 @@ def append(self, cpg: ChangePointGroup): "ChangePoints.append() can only be used such that time is monotonically increasing" ) - def extend(self, cps): + def extend(self, cps: list[ChangePointGroup]): errmsg = "ChangePoints.extend() takes as argument a list of ChangePointGroup objects." if not isinstance(cps, list): raise TypeError(errmsg) @@ -473,10 +483,10 @@ def select_metrics(self, m: list[str] | str): Get a new ChangePoints object holding only the given metric(s). If you think of a ChangePoints object as timestamps being rows, and - the metrics being columns, then this returns a single column. + the metrics being columns, then this returns a subset of the columns. Note: The internal data structure doesn't do anything to make this - request efficient. This will loop over all ChangePointGroups. + request efficient. This will loop over all ChangePointGroup s. Use ChangePointsByMetric if you need this to be fast. """ filtered = ChangePoints() @@ -509,7 +519,16 @@ def at_commit(self, sha: str): raise LookupError(sha) def pivot(self): - raise NotImplementedError() + """ + Return the same object as ChangePointsByMetric. + """ + by_metric = ChangePointsByMetric() + + for row in sorted(self._change_points, key=lambda cpg: cpg.time): + assert isinstance(row, ChangePointGroup) + # append() does the necessary shuffling into separate columns + by_metric.append(row) + return by_metric class ChangePointsByTime(ChangePoints): @@ -523,18 +542,6 @@ def from_dict(cls, cps: dict): "ChangePointsByTime doesn't accept a dict. Did you want ChangePointsByMetric.from_dict()?" ) - def pivot(self): - """ - Return the same object as ChangePointsByMetric. - """ - by_metric = ChangePointsByMetric() - - for row in sorted(self._change_points, key=lambda cpg: cpg.time): - assert isinstance(row, ChangePointGroup) - # append() does the necessary shuffling into separate columns - by_metric.append(row) - return by_metric - class ChangePointsByMetric(ChangePoints): """ @@ -588,7 +595,7 @@ def append(self, cpg: ChangePointGroup): self._change_points[metric] = [] self._change_points[metric].append(cpg.select_metrics(metric)) - def extend(self, cps): + def extend(self, cps: list[ChangePointGroup]): errmsg = "ChangePoints.extend() takes as argument a list of ChangePointGroup objects." if not isinstance(cps, list): raise TypeError(errmsg) @@ -724,10 +731,12 @@ def get_sides( def get_intervals(self, change_points: List[ChangePoint[GenericStats]]) -> List[slice]: """Returns list of slices of the series. Change points must be sorted by index.""" - assert all( + if not 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" + ): + raise ValueError("Change points must be sorted by index") + intervals = [ slice( 0 if i == 0 else change_points[i - 1].index, diff --git a/otava/main.py b/otava/main.py index 87fb4829..a6994e04 100644 --- a/otava/main.py +++ b/otava/main.py @@ -142,7 +142,7 @@ def update_grafana_annotations(self, test: GraphiteTestConfig, series: AnalyzedS logging.info(f"Found {len(old_annotations_for_test)} annotations") created_count = 0 - for metric_name, change_points in series.change_points.items(): + for metric_name, cpg_list in series.change_points.items(): path = test.get_path(series.branch_name(), metric_name) metric_tag = f"metric:{metric_name}" tags_to_create = ( @@ -171,13 +171,12 @@ def update_grafana_annotations(self, test: GraphiteTestConfig, series: AnalyzedS old_annotation_times = set((a.time for a in old_annotations if a.tags)) target_annotations = [] - for cp in change_points: - attributes = series.attributes_at(cp.index) - annotation_text = get_back_links(attributes) + for cpg in cpg_list: + annotation_text = get_back_links(cpg.attributes) target_annotations.append( Annotation( id=None, - time=datetime.fromtimestamp(cp.time, tz=pytz.UTC), + time=datetime.fromtimestamp(cpg.time, tz=pytz.UTC), text=annotation_text, tags=tags_to_create, ) @@ -242,18 +241,15 @@ def __get_bigquery(self) -> BigQuery: def update_postgres(self, test: PostgresTestConfig, series: AnalyzedSeries): postgres = self.__get_postgres() - for metric_name, change_points in series.change_points.items(): - for cpg in change_points: - cp = cpg[metric_name] - attributes = series.attributes_at(cp.index) - postgres.insert_change_point(test, metric_name, attributes, cpg) + for metric_name, cpg_list in series.change_points.items(): + for cpg in cpg_list: + postgres.insert_change_point(test, metric_name, cpg.attributes, cpg) def update_bigquery(self, test: BigQueryTestConfig, series: AnalyzedSeries): bigquery = self.__get_bigquery() - for metric_name, change_points in series.change_points.items(): - for cp in change_points: - attributes = series.attributes_at(cp.index) - bigquery.insert_change_point(test, metric_name, attributes, cp) + for metric_name, cpg_list in series.change_points.items(): + for cpg in cpg_list: + bigquery.insert_change_point(test, metric_name, cpg.attributes, cpg) def __maybe_create_slack_notifier(self): if not self.__conf.slack: diff --git a/otava/series.py b/otava/series.py index 15112287..372a6491 100644 --- a/otava/series.py +++ b/otava/series.py @@ -18,7 +18,7 @@ import logging from dataclasses import dataclass from datetime import datetime, timezone -from typing import Any, Dict, Iterable, List, Optional +from typing import Dict, Iterable, List, Optional from otava.analysis import ( TTestStats, @@ -137,9 +137,9 @@ class AnalyzedSeries: __series: Series options: AnalysisOptions - change_points: Dict[str, ChangePointGroup] - change_points_by_time: ChangePoints - change_points_timestamp: Any + change_points: ChangePointsByMetric + change_points_by_time: ChangePointsByTime + change_points_timestamp: ChangePointsByMetric def __init__( self, series: Series, options: AnalysisOptions, change_points: Dict[str, ChangePoint] = None diff --git a/tests/change_point_divisive_test.py b/tests/change_point_divisive_test.py index 45d44318..65b31bed 100644 --- a/tests/change_point_divisive_test.py +++ b/tests/change_point_divisive_test.py @@ -141,5 +141,5 @@ def test_get_intervals_requires_sorted_change_points(): 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"): + with pytest.raises(ValueError, match="Change points must be sorted by index"): tester.get_intervals(unsorted_cps) From f0233bd13a6c15402e460ea2590fc86fc1987952 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Wed, 17 Jun 2026 18:10:51 +0300 Subject: [PATCH 18/27] Add pytest-cov for code coverage --- pyproject.toml | 11 ++++ uv.lock | 140 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 83f79bec..5da20afb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,8 +101,19 @@ filterwarnings = [ "ignore::pytest.PytestCollectionWarning", ] +[tool.coverage.report] +# show_missing = true +skip_empty = true +skip_covered = true + + [tool.ruff] line-length = 100 [tool.isort] profile = "black" + +[dependency-groups] +dev = [ + "pytest-cov>=7.1.0", +] diff --git a/uv.lock b/uv.lock index d9f87eca..9bbbeaac 100644 --- a/uv.lock +++ b/uv.lock @@ -43,6 +43,11 @@ dev = [ { name = "tox" }, ] +[package.dev-dependencies] +dev = [ + { name = "pytest-cov" }, +] + [package.metadata] requires-dist = [ { name = "autoflake", marker = "extra == 'dev'", specifier = ">=2.3.1" }, @@ -72,6 +77,9 @@ requires-dist = [ ] provides-extras = ["dev"] +[package.metadata.requires-dev] +dev = [{ name = "pytest-cov", specifier = ">=7.1.0" }] + [[package]] name = "asn1crypto" version = "1.5.1" @@ -237,6 +245,124 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/31/28/d28211d29bcc3620b1fece85a65ce5bb22f18670a03cd28ea4b75ede270c/configargparse-1.7.1-py3-none-any.whl", hash = "sha256:8b586a31f9d873abd1ca527ffbe58863c99f36d896e2829779803125e83be4b6", size = 25607, upload-time = "2025-05-23T14:26:15.923Z" }, ] +[[package]] +name = "coverage" +version = "7.14.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/54/fd/0ab2772530e946e1be1abd0bc09e647ec9b02e88f0867857601fefca8953/coverage-7.14.1.tar.gz", hash = "sha256:30c08f7d90415aa98b3c990385dea2939b0da55f38515e5b369b83655f8523be", size = 920132, upload-time = "2026-05-26T20:41:36.783Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/92/69/0d2ef01ff4b8fcecd4cba920d11e92fa4f96ae412441d3b56a90a258e69b/coverage-7.14.1-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:3e3680291c4a1d0dadfa84a2c459576a4af5133abb617905714339a0c73138cf", size = 219722, upload-time = "2026-05-26T20:38:14.002Z" }, + { url = "https://files.pythonhosted.org/packages/f8/ae/9afdeaa31b9d9ce98124b6abf8bb49119bf71aecae04f8567c189d91299f/coverage-7.14.1-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:a5274669f37f2343635a347b91a60777621341ab3378e9c6ac9335eee704bddf", size = 220240, upload-time = "2026-05-26T20:38:17.424Z" }, + { url = "https://files.pythonhosted.org/packages/51/69/c998589871df7ea7dba865cc5ee32b5a3e1d47ba6c68ef91104c7c46fa5e/coverage-7.14.1-cp310-cp310-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:cfe5a5fec635799ef33428f1e5e61bafa45a92a96190ba731561ba558ccc214d", size = 246981, upload-time = "2026-05-26T20:38:19.266Z" }, + { url = "https://files.pythonhosted.org/packages/fc/10/1c7d04c13040dac531d21b712bbe08f902e6dd9b58f5d77875c4d030f8f2/coverage-7.14.1-cp310-cp310-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:62a9f70b52e0b5a95cfef4a5c5641b06983cadc5e538a3feeb5c00211f523ac2", size = 248812, upload-time = "2026-05-26T20:38:20.75Z" }, + { url = "https://files.pythonhosted.org/packages/c1/65/2a38a4607ef27cadcfbcee034dba5830ae2569f90144a0f4c7dbf47d30b0/coverage-7.14.1-cp310-cp310-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:3c18ebc343e15be53049b3a2dce38fe82d58f37e20ab9094b3a39c0aa4f6bb47", size = 250675, upload-time = "2026-05-26T20:38:22.159Z" }, + { url = "https://files.pythonhosted.org/packages/c9/a2/a446ed9752a4a59b79e0fb6cbb319f6facb2183045c0725462625e66f87e/coverage-7.14.1-cp310-cp310-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:b84ffdf877644e7096aa936991efeed873f7f3df57b9cd001312b7668ab08550", size = 252590, upload-time = "2026-05-26T20:38:23.63Z" }, + { url = "https://files.pythonhosted.org/packages/9e/fd/e81fbd7ba752365546e9842b1cbdaad3d6919d2a522c590aef16a281ec5e/coverage-7.14.1-cp310-cp310-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:e854312c4103f2ad4c0dc023b69b77ebfd2c89db5f86c4c94dc2353f9a92167e", size = 247691, upload-time = "2026-05-26T20:38:25.057Z" }, + { url = "https://files.pythonhosted.org/packages/53/35/f3c26fdaae9ea937d154ca4d372e5ea0a4167ff70d36c6074ac2eacb2f83/coverage-7.14.1-cp310-cp310-musllinux_1_2_aarch64.whl", hash = "sha256:c643734307300234fafa36bf2a040a7235f8f177ea1fd6ec1423aea6fb7b929f", size = 248716, upload-time = "2026-05-26T20:38:26.406Z" }, + { url = "https://files.pythonhosted.org/packages/2e/14/940b6c49551fd343e8507ee2b0ba7af5d0aa04ed5bf768285cb7c72a9884/coverage-7.14.1-cp310-cp310-musllinux_1_2_i686.whl", hash = "sha256:84ac9499e48700399a5dd0ea7085b5091961fec52c68d66b4ec0d3cf7f4441b1", size = 246721, upload-time = "2026-05-26T20:38:28.282Z" }, + { url = "https://files.pythonhosted.org/packages/aa/2c/40fc0634186c28292a662dff578866b3913983d6c375a3c2a74020938719/coverage-7.14.1-cp310-cp310-musllinux_1_2_ppc64le.whl", hash = "sha256:7f02d09f70776579b926d889a4c9c235070a1f47c40458aeaca563fae5acfdb5", size = 250533, upload-time = "2026-05-26T20:38:29.753Z" }, + { url = "https://files.pythonhosted.org/packages/de/e3/2c26bf1e811f9df991ff2a9bdddebdd13ee0665d564df7d05979f9146297/coverage-7.14.1-cp310-cp310-musllinux_1_2_riscv64.whl", hash = "sha256:ce66d8e46da2bb5ee313a745cbd2e391d319176c1f7a9451bfcd3a2fb920859b", size = 246990, upload-time = "2026-05-26T20:38:31.516Z" }, + { url = "https://files.pythonhosted.org/packages/a8/b0/060260ef56bd92363ebdce0c7095ce422b06e69aae71828efeca473ab1ca/coverage-7.14.1-cp310-cp310-musllinux_1_2_x86_64.whl", hash = "sha256:c912c259304cfb5ee584481cfb7ce1ff932b4d61e6c9140b8f19cb7b5ed82332", size = 247593, upload-time = "2026-05-26T20:38:33.065Z" }, + { url = "https://files.pythonhosted.org/packages/63/f3/501502046efeb0d6d94b5ca54941d95f1184183dd6bdb7f283985783bb4a/coverage-7.14.1-cp310-cp310-win32.whl", hash = "sha256:1238cb94638e610e972c60dac68e813f868dc7d6e982535270558443058d9d59", size = 222330, upload-time = "2026-05-26T20:38:35.36Z" }, + { url = "https://files.pythonhosted.org/packages/a0/5d/1bf99f2c558f128faf7906817ccbdb576ba815d3b41ce2ac1719b70a3663/coverage-7.14.1-cp310-cp310-win_amd64.whl", hash = "sha256:fc459e5d73be2d6332fcfe8dbf3d8994671fe33c700f4565988ecfa511547253", size = 223261, upload-time = "2026-05-26T20:38:37.196Z" }, + { url = "https://files.pythonhosted.org/packages/7d/d7/477ad149490e6cb849f28abea1dabb9c823cea72e7500c81b4240ce619c0/coverage-7.14.1-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:478b5bcd63c2e1357c5c7e16c070690df7b07f676b1c114d7b93e533c664309f", size = 219848, upload-time = "2026-05-26T20:38:38.715Z" }, + { url = "https://files.pythonhosted.org/packages/91/82/a5eb47257c50601bb7b9a9d2857c67b7a3a85ad74180eb2c98bb1fbe0ce5/coverage-7.14.1-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:a24a81f9715ee42ef59a316cc11611c98fe23920f7c81861315c9f3ff4a230f4", size = 220354, upload-time = "2026-05-26T20:38:40.232Z" }, + { url = "https://files.pythonhosted.org/packages/43/8b/78419b5391a5cb706b6544390507e469d83ffc9a8248b02c4011aceb9365/coverage-7.14.1-cp311-cp311-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:196a13319ad88d6d8ef5ab489ec4f44ddde2143c0c7d5b27786f6c3ffd56a7e1", size = 250771, upload-time = "2026-05-26T20:38:41.782Z" }, + { url = "https://files.pythonhosted.org/packages/77/63/e77aaacd491182210d639636b7a8bba23ffffa9b82aa3762da9431855fa9/coverage-7.14.1-cp311-cp311-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:3d452fd08b5c72c5167c93e6867b5c08500bd40f2a21e1e854a500550b6cc36f", size = 252683, upload-time = "2026-05-26T20:38:43.305Z" }, + { url = "https://files.pythonhosted.org/packages/65/1c/a022e3cfbec2ac241640003cb3a817e161d9c7f5aa9b49173756cdc03204/coverage-7.14.1-cp311-cp311-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:23bf7fa51ac02e07fc7c96849b82946da47ae862dc8f86d183b2a4864fc38129", size = 254791, upload-time = "2026-05-26T20:38:45.361Z" }, + { url = "https://files.pythonhosted.org/packages/61/d6/967e408aca4c1ceb88cb0cc677169110ae7f5995fb5eaf5fb1f5a1bb8f5d/coverage-7.14.1-cp311-cp311-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:bcaa50684dcaadfa599ac48f81103c756d791cfd85c97203d2217c593d48b860", size = 256748, upload-time = "2026-05-26T20:38:46.91Z" }, + { url = "https://files.pythonhosted.org/packages/b8/be/869188f7fe28638078ec479331ace6dc5f7b40b7153eb616f47ab79404d8/coverage-7.14.1-cp311-cp311-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:4ea1c034f95c9b056e856b794630b17f9fa3d57e4800ff1e503d3be0f9c9078c", size = 250907, upload-time = "2026-05-26T20:38:48.493Z" }, + { url = "https://files.pythonhosted.org/packages/07/aa/adb7d3b4278d690e68703abcd76ab1b948242e3668d921711551b78f9ddb/coverage-7.14.1-cp311-cp311-musllinux_1_2_aarch64.whl", hash = "sha256:c7e057326434e441306226fbeb5d1aaf14a2637efe97ba668306635835f32ad7", size = 252483, upload-time = "2026-05-26T20:38:50.074Z" }, + { url = "https://files.pythonhosted.org/packages/43/61/331c74103c62dcb0c4b9b3a0de9a61aca016208b0a90f109592a9f9ecc28/coverage-7.14.1-cp311-cp311-musllinux_1_2_i686.whl", hash = "sha256:59baf88468dbc8d63b1887afd92bda52e40bb1561696e5819670601403810cec", size = 250545, upload-time = "2026-05-26T20:38:51.613Z" }, + { url = "https://files.pythonhosted.org/packages/f6/b6/c5dae3c104d89be04828f61810e6b3473825482e4c288cc4ed04553e08ae/coverage-7.14.1-cp311-cp311-musllinux_1_2_ppc64le.whl", hash = "sha256:d34d75f892b3ab73ba11cab5442cce7b3e168fd64162b16f0e1e0d09c508edef", size = 254310, upload-time = "2026-05-26T20:38:53.503Z" }, + { url = "https://files.pythonhosted.org/packages/ad/a1/2b9d5863e3b83c01ad8199e3c597802fbb3a9dc90b058885804c20296d31/coverage-7.14.1-cp311-cp311-musllinux_1_2_riscv64.whl", hash = "sha256:3a56abc20a472baf0304c455721bc601477440d28ecfde8a03dde79ede07e0df", size = 250266, upload-time = "2026-05-26T20:38:55.414Z" }, + { url = "https://files.pythonhosted.org/packages/7f/5e/0e511fbdb269359be26fe678a1c3fa1f2aa2a01573cc3f54268c8d6d4797/coverage-7.14.1-cp311-cp311-musllinux_1_2_x86_64.whl", hash = "sha256:6a3cb83d1552c0cd1b4906655b6a33fd4a8473229633a901c6b73bf86914dee9", size = 251174, upload-time = "2026-05-26T20:38:57.141Z" }, + { url = "https://files.pythonhosted.org/packages/85/10/e55307b622b3dd9671cb321824502dc10f93e72f2802b9946159a8edadeb/coverage-7.14.1-cp311-cp311-win32.whl", hash = "sha256:10274a1fbeb8ec5d72966e17bb198a3104257aca4ac09d98667c5f8aca8c8548", size = 222354, upload-time = "2026-05-26T20:38:58.727Z" }, + { url = "https://files.pythonhosted.org/packages/71/cf/107421693cfb71e4f1ca5bf70443f64d4161878068d07a3e51c7ad21d17b/coverage-7.14.1-cp311-cp311-win_amd64.whl", hash = "sha256:87ebdf787d4888e3f3f2d523eadc6e18c6d18c6d0eb173801a189641627fb37e", size = 223290, upload-time = "2026-05-26T20:39:00.413Z" }, + { url = "https://files.pythonhosted.org/packages/b8/1d/3e3644585eb29e9dafefb19555078529a4d7cce12bd21929664eea989277/coverage-7.14.1-cp311-cp311-win_arm64.whl", hash = "sha256:dd34767fa19848d35659ffc0a75314f58c7af3f1cd87ec521e8292a1238398a3", size = 221953, upload-time = "2026-05-26T20:39:02.159Z" }, + { url = "https://files.pythonhosted.org/packages/3d/b7/bdbb725ba02c5b42825b200c940f38b7a54fcad24627b7192f78f8110d76/coverage-7.14.1-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:a06c76364a9360e33d6d23769aefdf7f66f38e2ffb60ceb1baaa4989d83b695c", size = 220022, upload-time = "2026-05-26T20:39:03.702Z" }, + { url = "https://files.pythonhosted.org/packages/72/81/fdc0898a55c6219223291ec1a1fe89966ef212ce82276aa0899df84b5de0/coverage-7.14.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:fad54e871165f6ec2f536063ac74c3104508a12963e64072ba44bd822de52b0c", size = 220379, upload-time = "2026-05-26T20:39:05.381Z" }, + { url = "https://files.pythonhosted.org/packages/de/72/de048c4a25e13bce59ac6a339351c10bdf2515e07459afcdaf04dc3143a2/coverage-7.14.1-cp312-cp312-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:84b535f00655ecafe1d929d1fb00ed5d6fa3051ea643ab2c161a3887b86f294b", size = 251888, upload-time = "2026-05-26T20:39:07.367Z" }, + { url = "https://files.pythonhosted.org/packages/28/30/300c343f68beb9d4cbb64ec81e58c5b6b80b56927f72d2b38654ac26e013/coverage-7.14.1-cp312-cp312-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:6b6b0853b895fe0e98cbfc580d1ec3393d9302b4b1e96a77b3f5c91fdab899e6", size = 254624, upload-time = "2026-05-26T20:39:09.037Z" }, + { url = "https://files.pythonhosted.org/packages/b1/ed/7b25642496e8170b6bac14adce00537c6e5fa2d586159401a4de3e8b49e6/coverage-7.14.1-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:442cc9c952b2df400cda54bb04ab87330cf2cd08a8692cbbea36773531eb6f37", size = 255739, upload-time = "2026-05-26T20:39:10.889Z" }, + { url = "https://files.pythonhosted.org/packages/7f/a2/abd210b8c4e29c24e4624916db97bb519097a91034aaeb767f937e7da794/coverage-7.14.1-cp312-cp312-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:8270544c361ed405a27a060dbc9ed2c124b084d96dfdc2d9a2510482aef981ad", size = 257998, upload-time = "2026-05-26T20:39:12.722Z" }, + { url = "https://files.pythonhosted.org/packages/7f/24/7c50beed3792fe62f6ce0545c6686ce83379719e2c0276179333d97eae92/coverage-7.14.1-cp312-cp312-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:48b283b1dd6372e8de2a7a9a4c4d5dc06f4d4fd209b876f3c88a7a205a0c8f84", size = 252296, upload-time = "2026-05-26T20:39:14.259Z" }, + { url = "https://files.pythonhosted.org/packages/15/05/0f874628ebcbfc77ead559ff210281ef06a97db08481832e7dd39274a135/coverage-7.14.1-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:5b0c99ba93a07d56f6df340bb79be53202a082b2fdb81bfe6190b741a3470d54", size = 253658, upload-time = "2026-05-26T20:39:15.923Z" }, + { url = "https://files.pythonhosted.org/packages/99/6f/ca6ad067364b337ef997802115e7ecad2abd2248b05471464b0dea02b4d4/coverage-7.14.1-cp312-cp312-musllinux_1_2_i686.whl", hash = "sha256:e471bc5769ff073b058cfadb0d736b56ce067c8560eabeb0da88462df98c23e7", size = 251803, upload-time = "2026-05-26T20:39:17.537Z" }, + { url = "https://files.pythonhosted.org/packages/c0/30/b9b4d377cd9f40baf228068f5a81faf8450c6228503011bd499708483a50/coverage-7.14.1-cp312-cp312-musllinux_1_2_ppc64le.whl", hash = "sha256:f497a1ea81d4cd7c10ddcaa685135b9aabd291af3d55775a9ddf3cb7a364cdd9", size = 255873, upload-time = "2026-05-26T20:39:19.414Z" }, + { url = "https://files.pythonhosted.org/packages/3c/21/7c721a9e5e6bb88547d30a787aefb97512d3f54c1324c7488d9b3743f7f9/coverage-7.14.1-cp312-cp312-musllinux_1_2_riscv64.whl", hash = "sha256:2222be86d0b54f5dd5a38f45f17f315f737245e857bf0bdedc70734f84a13c02", size = 251372, upload-time = "2026-05-26T20:39:21.169Z" }, + { url = "https://files.pythonhosted.org/packages/9d/8c/f8ae5a2200130e1503cd7661a6cd3b2b7bacef98277fbf3571fb13f8b766/coverage-7.14.1-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:85e85586565842f6932abebd4c18bcb1074223dc0b3576e7d173ca710622813a", size = 253245, upload-time = "2026-05-26T20:39:23.097Z" }, + { url = "https://files.pythonhosted.org/packages/34/62/70a9024672a5f6910517d9628c52c9afbdd3cf8f46426af52bb148a56fff/coverage-7.14.1-cp312-cp312-win32.whl", hash = "sha256:4a28fd227808366b196a75476dced2eb35b351d6766ba9c858dc93319e87f4f1", size = 222567, upload-time = "2026-05-26T20:39:24.868Z" }, + { url = "https://files.pythonhosted.org/packages/f6/81/8b7cd386839b039ebe1855733b9f9449a8dec5d79564018234f185a7fa70/coverage-7.14.1-cp312-cp312-win_amd64.whl", hash = "sha256:54acdb6674a4661768d7bf7db32dfb9f46ab1d764f8aba6df75ce1a6a088724e", size = 223372, upload-time = "2026-05-26T20:39:26.603Z" }, + { url = "https://files.pythonhosted.org/packages/ae/ba/b44d472022f620d289d95fa830143235c0c36461c6f2437ea8d51e5481ed/coverage-7.14.1-cp312-cp312-win_arm64.whl", hash = "sha256:99cd41ff91afd94896fea3bc002706b6ae4ce95727d06e4a0f39c0a8d8bd8b1a", size = 221989, upload-time = "2026-05-26T20:39:28.242Z" }, + { url = "https://files.pythonhosted.org/packages/8a/9e/5f6d56327c62b185225d145191c607e07515294a0aa6338e58805cd4a5ac/coverage-7.14.1-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:be9f2c802dcfce3f71298303aa5dad0dce440a76c52f2f60dacd8656dab78793", size = 220044, upload-time = "2026-05-26T20:39:29.902Z" }, + { url = "https://files.pythonhosted.org/packages/75/92/e82aca356744cbbc0f77a0b623e38918c1872361963413a3bab5d0340393/coverage-7.14.1-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:6223a72fd0e4c7156353ec0f08a5f93623e1d3034d0e2683b9bb8ea674131b1d", size = 220412, upload-time = "2026-05-26T20:39:31.561Z" }, + { url = "https://files.pythonhosted.org/packages/27/c9/385bde0bf7ed0f4bf3a7ee5367060a86b5d218718cfd6fb943c0f836b34f/coverage-7.14.1-cp313-cp313-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:7279d2110a28cebc738b6459ecda2771735a4c18465fbbd36b3288fe5ed92247", size = 251412, upload-time = "2026-05-26T20:39:33.337Z" }, + { url = "https://files.pythonhosted.org/packages/51/8c/23faf6a2343a0d17f960a4bd56c43bc7eb4cf312f774dd6ceebd82c7d8fc/coverage-7.14.1-cp313-cp313-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:9eeb3fcbc13ba40dfbdb22d01d196a28e9cef9ed4c29b60061a1e0e823a9929d", size = 254008, upload-time = "2026-05-26T20:39:35.009Z" }, + { url = "https://files.pythonhosted.org/packages/42/06/36f4aa9ca8a815e6036156e80706a67828bb97bd826948244f6996dda957/coverage-7.14.1-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:5f0cfc27c539f07cf5c0a4cfe211d0b6cae039f8f40526dbaa71944e64b50a7b", size = 255241, upload-time = "2026-05-26T20:39:36.71Z" }, + { url = "https://files.pythonhosted.org/packages/ca/79/95266316352f90f6b1c6736bb413302edfde2453fb32422d3911642691b3/coverage-7.14.1-cp313-cp313-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:221c70f316241a78e77e607c227cefc8808d4e08f28d99c04f35694690e940be", size = 257373, upload-time = "2026-05-26T20:39:38.412Z" }, + { url = "https://files.pythonhosted.org/packages/e3/9c/58316d1f66c488b5fca8a0eb3e98348807813efa8a0d0833b9021be27488/coverage-7.14.1-cp313-cp313-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:da028256b04ec30e5e0114b6f76172938c313991f0a2d3d894271315cf5d5e43", size = 251635, upload-time = "2026-05-26T20:39:40.268Z" }, + { url = "https://files.pythonhosted.org/packages/ef/5a/ca2398a568e16fed7bb713e84ba3603a7164fb65779abe645c565ec890d5/coverage-7.14.1-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:76a085d7005236a767e3426148b2c407e53ad61695c562f8a81da2d373324901", size = 253373, upload-time = "2026-05-26T20:39:42.145Z" }, + { url = "https://files.pythonhosted.org/packages/6e/2c/0396562c32deaebe7be51d865b3a41e9a87d7561acafe1a28f53b07e019a/coverage-7.14.1-cp313-cp313-musllinux_1_2_i686.whl", hash = "sha256:b553d04b5e778a8e56d57eb134aff42a92718ecba45e79c4764ecfa40efd92ff", size = 251341, upload-time = "2026-05-26T20:39:43.907Z" }, + { url = "https://files.pythonhosted.org/packages/fd/8f/a94f9221184c9cae1ee115820e3798e48b6b17777a9f19e46fb9a0c8dc74/coverage-7.14.1-cp313-cp313-musllinux_1_2_ppc64le.whl", hash = "sha256:46f714d2fb8ae2f4f29f23ada7f1e79b759fff5a70f94a1dac23af204c3ec9e4", size = 255497, upload-time = "2026-05-26T20:39:46.166Z" }, + { url = "https://files.pythonhosted.org/packages/71/69/505d70e47db1eaebcd002c39759707621ef184cd6b1ae084d9f41293f323/coverage-7.14.1-cp313-cp313-musllinux_1_2_riscv64.whl", hash = "sha256:1896f5e19ff3f0431c7ce2172adc54890fd97f86b59ced8ca1649145d9ffe35d", size = 251159, upload-time = "2026-05-26T20:39:48.03Z" }, + { url = "https://files.pythonhosted.org/packages/e0/aa/58681c383aa33a9d2ed40a02d7a22fbf780d1fa4d575396365777828198c/coverage-7.14.1-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:62fd185ef9df3c33d1c8178c5af105f762afbad96038de9a4ae100aa6297ca33", size = 252934, upload-time = "2026-05-26T20:39:49.872Z" }, + { url = "https://files.pythonhosted.org/packages/eb/fd/11c928cd6bdffc7074bb5965c173d9ebf517fb00205e1da524b98d29ef92/coverage-7.14.1-cp313-cp313-win32.whl", hash = "sha256:ab4af6352741a604c431c6072fce5bee33bf0f20dc7a56618d6bf6bb89e9810c", size = 222584, upload-time = "2026-05-26T20:39:51.68Z" }, + { url = "https://files.pythonhosted.org/packages/6f/92/fb416fc26d340dcba19518c418d6048e913186e17243982c5e435e41fa7a/coverage-7.14.1-cp313-cp313-win_amd64.whl", hash = "sha256:7af486dabe8954d03b087f0021540897afe084f04e16ff5579e08cc46f871416", size = 223394, upload-time = "2026-05-26T20:39:53.472Z" }, + { url = "https://files.pythonhosted.org/packages/73/c6/02d56e3867972f77d5036de924643f26c056e848f00452cafb4dbc3c29b4/coverage-7.14.1-cp313-cp313-win_arm64.whl", hash = "sha256:2224f89ffd0c5605ccce1ed7a584da162bc7c55f601ab1c946bc9de31a486b42", size = 222015, upload-time = "2026-05-26T20:39:55.374Z" }, + { url = "https://files.pythonhosted.org/packages/4d/9e/fcc77914050df73f7662fa1f00902774c79c075a8388ab334074574bf77e/coverage-7.14.1-cp313-cp313t-macosx_10_13_x86_64.whl", hash = "sha256:de286598cc65d2b489411174b1faec2f5a7775fb3201fd925db2a76b4030f37d", size = 220733, upload-time = "2026-05-26T20:39:57.189Z" }, + { url = "https://files.pythonhosted.org/packages/f7/67/2963cbdaf5cbadec44efa3a1e39eaa1f02df4079585f05387607a221e126/coverage-7.14.1-cp313-cp313t-macosx_11_0_arm64.whl", hash = "sha256:042c46ded7c288aeb07cf14a28b6c1e10b78fcba40171c3fa1e939377eeef0b5", size = 221086, upload-time = "2026-05-26T20:39:59.019Z" }, + { url = "https://files.pythonhosted.org/packages/c8/c5/8701645574e11881f2f47d8930f98bc48b5d43b25eb5b4430dfc4a2f9f48/coverage-7.14.1-cp313-cp313t-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:f4ddbe407477f04c45115d1a4e5bc480f753553b534d338d4c3358b1cdd0ea52", size = 262381, upload-time = "2026-05-26T20:40:00.822Z" }, + { url = "https://files.pythonhosted.org/packages/7c/28/7a64d73598263e0c5abd5084211a8474488d31b3c552ff531c719dfcff62/coverage-7.14.1-cp313-cp313t-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:d13e6725992e2d2fd7d81d4f5241952d13740121dfd501da09201be39b2c003a", size = 264458, upload-time = "2026-05-26T20:40:02.506Z" }, + { url = "https://files.pythonhosted.org/packages/fa/d8/4969179db9f7eb4df218e69540adf829d1c835f59452513d065d15446802/coverage-7.14.1-cp313-cp313t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:f747dc8edcfe740130f28f32f3995e955494285717e86ee25af51db2219df08a", size = 266884, upload-time = "2026-05-26T20:40:04.421Z" }, + { url = "https://files.pythonhosted.org/packages/a6/78/a45d5794dbc9bafd97afc96a4377c86c7820d78b6cf51b89bc1d4e919275/coverage-7.14.1-cp313-cp313t-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:ced2f09ef276fd58611a1ef502164ad266d2b75174e5a40cabbdb4033f9f6cf2", size = 268022, upload-time = "2026-05-26T20:40:06.298Z" }, + { url = "https://files.pythonhosted.org/packages/21/cb/4f5e354e9e3e67af96bd4e57113e6db6b22298c7168b13eec408a549903d/coverage-7.14.1-cp313-cp313t-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:b84800013769a78ccb9ef4659402e26d06867e337b61ec365f77ad008adea80e", size = 261631, upload-time = "2026-05-26T20:40:08.226Z" }, + { url = "https://files.pythonhosted.org/packages/ec/49/eced49af4cb996d5d8b7e94e736175c513e4facd3398507b89892b4326d8/coverage-7.14.1-cp313-cp313t-musllinux_1_2_aarch64.whl", hash = "sha256:ea8cd6ca0ee9f616aaef3afc6882e32c2cbf18b00d96313ffd76af650574034d", size = 264443, upload-time = "2026-05-26T20:40:10.137Z" }, + { url = "https://files.pythonhosted.org/packages/f1/d8/5603a88a7c5913a6b54f6cb1a8c46f7b39cbb30f27cd3f492908da09b2d7/coverage-7.14.1-cp313-cp313t-musllinux_1_2_i686.whl", hash = "sha256:aa5e304a873fabddc11e484e9b6b738bd38bd7bed17b09aa84eecf5332e8b8bb", size = 262069, upload-time = "2026-05-26T20:40:11.999Z" }, + { url = "https://files.pythonhosted.org/packages/f0/59/2ae3cb79da554a06c8619d6c88ea19dd1e4aed4b834b6a83bb1fa243bdc5/coverage-7.14.1-cp313-cp313t-musllinux_1_2_ppc64le.whl", hash = "sha256:5a1c5215be81035e629d5bc756650634d0bf31991038db7a0eccb90f025ce16d", size = 265780, upload-time = "2026-05-26T20:40:13.858Z" }, + { url = "https://files.pythonhosted.org/packages/af/5f/b130c1dc999031f2648bd25317fbce505ad8d5562079b4ed81e736a84967/coverage-7.14.1-cp313-cp313t-musllinux_1_2_riscv64.whl", hash = "sha256:79058c47dae6788504b5effb319961bcd72d7240551464b91d474bc0ed186d69", size = 260970, upload-time = "2026-05-26T20:40:16.142Z" }, + { url = "https://files.pythonhosted.org/packages/87/d1/ec13ccddeb48ec963bdfa72a11224bac2584bd045ba13beca82f8113e9c7/coverage-7.14.1-cp313-cp313t-musllinux_1_2_x86_64.whl", hash = "sha256:370c5afae3fa0658e11694a32b24c2778f6bc2d17718121f94ee185e69f26b54", size = 263157, upload-time = "2026-05-26T20:40:18.382Z" }, + { url = "https://files.pythonhosted.org/packages/cf/c2/cd91ead503045161092d3845f7bb95ea2f25131ce96d3e314dd835d91b9c/coverage-7.14.1-cp313-cp313t-win32.whl", hash = "sha256:3758dd0a7f1fa57365ef2e781df0f0731d38b6e3772259d13dae4bd8a958d4b1", size = 223259, upload-time = "2026-05-26T20:40:20.381Z" }, + { url = "https://files.pythonhosted.org/packages/71/9f/1e28d97e6bd2c76b07f38b7c02870f1371255ff6717f54eca578fcbbdd0e/coverage-7.14.1-cp313-cp313t-win_amd64.whl", hash = "sha256:6ff665fb023a77386fe11685190cee1f60a7d635994a30d9b0a061533d470fce", size = 224320, upload-time = "2026-05-26T20:40:22.316Z" }, + { url = "https://files.pythonhosted.org/packages/a9/e0/d936e908f0e1efa55e52b91e01b52f1055cef5e1ab2718493390ed8e2fb8/coverage-7.14.1-cp313-cp313t-win_arm64.whl", hash = "sha256:17a5a241e5997621a956a7f402a7433ef4221e5152809b785bec79e2323799f1", size = 222577, upload-time = "2026-05-26T20:40:24.894Z" }, + { url = "https://files.pythonhosted.org/packages/d6/34/fc2f101b151af3799a101f0550b0454aa008afdc0add677394ec4aa8ea10/coverage-7.14.1-cp314-cp314-macosx_10_15_x86_64.whl", hash = "sha256:d5ed429d0b8edaac649e889b4ffcedb6c80b06629a3f93050e3dddfb99235bee", size = 220091, upload-time = "2026-05-26T20:40:27.249Z" }, + { url = "https://files.pythonhosted.org/packages/3d/a7/1ebae2ab5b961b5c79bb09fe7b3ac99edb190d8be4a8c510b2cf66f46468/coverage-7.14.1-cp314-cp314-macosx_11_0_arm64.whl", hash = "sha256:8011224a62280e50dab346960c03cf47aca1a1e09e608c0fb33fd6e0cc8e9500", size = 220421, upload-time = "2026-05-26T20:40:30.084Z" }, + { url = "https://files.pythonhosted.org/packages/5e/90/92aca9cf0acc95123c96cd1eb1f08917897a7f5dee01e15738922971ec31/coverage-7.14.1-cp314-cp314-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:12c42ec1e14f553c4f817e989365982e646e27211f10a0f717855b94a79c8906", size = 251466, upload-time = "2026-05-26T20:40:32.542Z" }, + { url = "https://files.pythonhosted.org/packages/26/2b/78048cbe3b999f6cbf9cc0d90abba6a88a3e0863a8c1c6cbc762f3f8802f/coverage-7.14.1-cp314-cp314-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:06144cd511cf2624873a035c5069cf297144f6e77a73ee3d7a55b605ec5efb42", size = 253973, upload-time = "2026-05-26T20:40:34.473Z" }, + { url = "https://files.pythonhosted.org/packages/8e/21/c2e33b29d1cfde484a19d437afc343c6cd30b08d78cbbf9f5aff14e57b2b/coverage-7.14.1-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:a311d8e1da24be5c1ccf85cbfb06315dbaa1703d5a1eab3f6432c72b837917c8", size = 255318, upload-time = "2026-05-26T20:40:38.154Z" }, + { url = "https://files.pythonhosted.org/packages/8e/ee/aad2f108d63b769121005302f16bf66db8625c88ceaba466942e09a2607e/coverage-7.14.1-cp314-cp314-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:c79cead5b5bc584d9c71451cb984d0e3a84e0c0937379c8efcbf27c8d661b851", size = 257633, upload-time = "2026-05-26T20:40:40.164Z" }, + { url = "https://files.pythonhosted.org/packages/c2/f8/11a2c29b4fd76d9849f81d0bb812ec0017a9396df3217214e38934a8c837/coverage-7.14.1-cp314-cp314-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:dcbf65f1f66a26cdd88c35cf68fb4729c5d1cd2e88added72420541dfb212034", size = 251488, upload-time = "2026-05-26T20:40:42.631Z" }, + { url = "https://files.pythonhosted.org/packages/c9/b8/9a5820de4b8ac2b71d85e3b5fb49108d7469c665f0e2ad0dd7569023e305/coverage-7.14.1-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:fd86572566fb40189a8260446158235159bc7a82dfbc87a3b39cf4fb57fcec1c", size = 253329, upload-time = "2026-05-26T20:40:45.208Z" }, + { url = "https://files.pythonhosted.org/packages/6b/ff/f33e4823667e27548e8fd8df44217515303f9808d0ff29817db56f87d990/coverage-7.14.1-cp314-cp314-musllinux_1_2_i686.whl", hash = "sha256:7771b601718fdde84832c3a434ca9bbf4ae9adbc49d84198b4110700c3c77c36", size = 251291, upload-time = "2026-05-26T20:40:47.502Z" }, + { url = "https://files.pythonhosted.org/packages/68/9b/489db0ebb209054766b90a9014a45f6d26eb724c02ec21311c3733b5a644/coverage-7.14.1-cp314-cp314-musllinux_1_2_ppc64le.whl", hash = "sha256:39b21e212c55af06fa375e3dbf90a8a8e38792f3a910c580066d23563830ddd5", size = 255564, upload-time = "2026-05-26T20:40:49.372Z" }, + { url = "https://files.pythonhosted.org/packages/27/b5/16bc2d4c2409b23c7737edb68c83bc89e345f378050549fe1d75ac7d34d5/coverage-7.14.1-cp314-cp314-musllinux_1_2_riscv64.whl", hash = "sha256:f2302660e32562a532b442480121aef8aa61a5bdb20b30bf0adab29f10a5a4b4", size = 251107, upload-time = "2026-05-26T20:40:51.677Z" }, + { url = "https://files.pythonhosted.org/packages/7d/0c/2629997469a00cd069d588a41c9dc887610f2775ae89d250c4791e65272a/coverage-7.14.1-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:03a6f93c1ec3b7f2e77b5dbcc5573a2c21f12529a5c6bbe0f16f72303cc2fa4d", size = 252764, upload-time = "2026-05-26T20:40:54.267Z" }, + { url = "https://files.pythonhosted.org/packages/d2/ee/f78d63c8f079e0d7211c7e2401fa17e311514534ba61bae03e4b287ce4ab/coverage-7.14.1-cp314-cp314-win32.whl", hash = "sha256:8a3ce026d73290f42f08dafecbd82c193a74df280461fbf97300fec51fd133ee", size = 222837, upload-time = "2026-05-26T20:40:56.496Z" }, + { url = "https://files.pythonhosted.org/packages/dc/b9/be539854f93a70dfbeec69117f33ec70dc42ff0b65b5b07ab8d40d04228e/coverage-7.14.1-cp314-cp314-win_amd64.whl", hash = "sha256:114c95ef29302423b87d159075805f4ab973254a2638a5d7d046c94887cc87d7", size = 223650, upload-time = "2026-05-26T20:40:58.351Z" }, + { url = "https://files.pythonhosted.org/packages/fe/9e/24e2842fef40f35ac82ba3a7719c8023d011bf3bf652d0675316a9d088a1/coverage-7.14.1-cp314-cp314-win_arm64.whl", hash = "sha256:a07891c3f4805442b31b71e84ba3cf29ed1aa9a428284e06deeb4b23e5b46343", size = 222218, upload-time = "2026-05-26T20:41:00.321Z" }, + { url = "https://files.pythonhosted.org/packages/0a/1d/ac0a9df5fe31c1e8bdd658074905fc12844a05c1a7e3fdb8417e97c31e23/coverage-7.14.1-cp314-cp314t-macosx_10_15_x86_64.whl", hash = "sha256:1101a5ebb083aecb625ebb6209d4105b58f647b093cb2dc8122d7b33f743cfe1", size = 220822, upload-time = "2026-05-26T20:41:02.281Z" }, + { url = "https://files.pythonhosted.org/packages/32/cf/f964fd9aff20323f9f1a726c97135f8a76bcd87b92dad141a456a43f3c64/coverage-7.14.1-cp314-cp314t-macosx_11_0_arm64.whl", hash = "sha256:851b9e1e4e8a4608e77c79714b2e77c0970d2ed7202a05e92ae407817481887b", size = 221084, upload-time = "2026-05-26T20:41:04.593Z" }, + { url = "https://files.pythonhosted.org/packages/d8/5e/7e5ef2aba844de2b80d678619fcf0841b42e3f37f16411226f3fe4c1016f/coverage-7.14.1-cp314-cp314t-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:d5b89cdfb2ee051b71e8c3c70bd81a9eff81100f736a269136fe1a68efe00474", size = 262454, upload-time = "2026-05-26T20:41:06.641Z" }, + { url = "https://files.pythonhosted.org/packages/64/62/75809bded87015cc4935524218a2a8ed8dd1a8498bfed30a2f4f7a4b4d34/coverage-7.14.1-cp314-cp314t-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:0177614a0370f227888b4e436a7c55686d6a9f90eb1ade2b624ba685a1686e86", size = 264578, upload-time = "2026-05-26T20:41:08.556Z" }, + { url = "https://files.pythonhosted.org/packages/f3/42/d33392dc14633525012d2d504fa1a33b05538bf535f5c1d64675e5754b78/coverage-7.14.1-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:2d69af5dea2de76fc485a83032a630523f985198b7e25be901ec60181587b01e", size = 266981, upload-time = "2026-05-26T20:41:10.824Z" }, + { url = "https://files.pythonhosted.org/packages/2a/49/0157c4428c2aca7f1e09d5565930586fd5ae36f1655f08b0daa7cf1fcae1/coverage-7.14.1-cp314-cp314t-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:35ab22d91de736e8966b980dc355cbcdd2c6dbbcfe275f9a2991bc8a91b3df65", size = 268112, upload-time = "2026-05-26T20:41:12.966Z" }, + { url = "https://files.pythonhosted.org/packages/96/26/86b9ce71f4092b1ed325ce1421698081df1286b833400b6836912834d6e0/coverage-7.14.1-cp314-cp314t-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:357d4e32935c36588aaba057d734fa32428c360c9fc2e4442afbf1b646beee6e", size = 261558, upload-time = "2026-05-26T20:41:15Z" }, + { url = "https://files.pythonhosted.org/packages/20/4c/c311210c5472cf5401d8422b0d7812cdd520f24417673afabda6c323faca/coverage-7.14.1-cp314-cp314t-musllinux_1_2_aarch64.whl", hash = "sha256:51bd64741cc6fa065abd300ede1afe5a5291ece9c31da8b24884deda48bcc3f8", size = 264447, upload-time = "2026-05-26T20:41:17.369Z" }, + { url = "https://files.pythonhosted.org/packages/fb/71/59513f8710ed3e6b0ac0a050a5b7e977bb9c9e880354863b5d00d8809256/coverage-7.14.1-cp314-cp314t-musllinux_1_2_i686.whl", hash = "sha256:9132cd363a68a4c3daa7c8704a654b1e39d3360f6f5b8ddd470608a945236c07", size = 262048, upload-time = "2026-05-26T20:41:19.309Z" }, + { url = "https://files.pythonhosted.org/packages/84/8d/bceed32dc494f5bbf50f775cd2e78ca814953942b5ea28d3c1c3ac316f14/coverage-7.14.1-cp314-cp314t-musllinux_1_2_ppc64le.whl", hash = "sha256:07c6290b1697b862c0478eab545eec949a0d0e4d6d03497f446d706da3b4f2de", size = 265781, upload-time = "2026-05-26T20:41:21.559Z" }, + { url = "https://files.pythonhosted.org/packages/e7/c5/9348fe40dbfd4991aaf78df2c6c3098bfb2cc834d1fd362a64b4efef855a/coverage-7.14.1-cp314-cp314t-musllinux_1_2_riscv64.whl", hash = "sha256:5ea0c297e27133853b4d8a3eb799bff5a2dbd9f2f41537a240d337ac9b4df890", size = 260896, upload-time = "2026-05-26T20:41:23.428Z" }, + { url = "https://files.pythonhosted.org/packages/ca/92/1ea0f03929da7cf87206b1fa24f4c8e9c158be0455481af29ec0a1f3503f/coverage-7.14.1-cp314-cp314t-musllinux_1_2_x86_64.whl", hash = "sha256:01b7733daad0237daa01ef80fe2dfceffc911e6a17fa7b55d14aa8214eaaaecd", size = 263214, upload-time = "2026-05-26T20:41:25.419Z" }, + { url = "https://files.pythonhosted.org/packages/f6/a9/b2493c054c0e01a643266742ab45e15744e60743f9260cd930c7142b1124/coverage-7.14.1-cp314-cp314t-win32.whl", hash = "sha256:6adc5a36984624a70bf11d7184e20fa0a49aa7c47ffab43804106a1a695ea22e", size = 223624, upload-time = "2026-05-26T20:41:27.795Z" }, + { url = "https://files.pythonhosted.org/packages/fc/bd/3e1e6a57fccd2d7c83fcdf338e93ba98eb85c6e877dd34731ac585375490/coverage-7.14.1-cp314-cp314t-win_amd64.whl", hash = "sha256:ddf799247318f34dbcd2efa8c95a8d0642674e926bb1774cf9b63dfd2a389d1c", size = 224728, upload-time = "2026-05-26T20:41:30.098Z" }, + { url = "https://files.pythonhosted.org/packages/bb/d7/31066cf1d2f0c6c797fce911bcfa01dd35642dc6da992a950256097c5860/coverage-7.14.1-cp314-cp314t-win_arm64.whl", hash = "sha256:145986fe66647eb489f18d9a997567a3fd358584c4b5a808769113abc07466af", size = 222752, upload-time = "2026-05-26T20:41:32.123Z" }, + { url = "https://files.pythonhosted.org/packages/8a/3c/1a983b9a745d7f83d53f057bcc5bf79ba6a2bbc08266b3f0c7d6fe630c9b/coverage-7.14.1-py3-none-any.whl", hash = "sha256:a252f21c27e38347e60111a3266b03827422a7d5525951aceee313aa68bab1d2", size = 211815, upload-time = "2026-05-26T20:41:34.078Z" }, +] + +[package.optional-dependencies] +toml = [ + { name = "tomli", marker = "python_full_version <= '3.11'" }, +] + [[package]] name = "dateparser" version = "1.2.0" @@ -895,6 +1021,20 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/33/29/e756e715a48959f1c0045342088d7ca9762a2f509b945f362a316e9412b7/pytest_benchmark-5.2.3-py3-none-any.whl", hash = "sha256:bc839726ad20e99aaa0d11a127445457b4219bdb9e80a1afc4b51da7f96b0803", size = 45255, upload-time = "2025-11-09T18:48:39.765Z" }, ] +[[package]] +name = "pytest-cov" +version = "7.1.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "coverage", extra = ["toml"] }, + { name = "pluggy" }, + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b1/51/a849f96e117386044471c8ec2bd6cfebacda285da9525c9106aeb28da671/pytest_cov-7.1.0.tar.gz", hash = "sha256:30674f2b5f6351aa09702a9c8c364f6a01c27aae0c1366ae8016160d1efc56b2", size = 55592, upload-time = "2026-03-21T20:11:16.284Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/9d/7a/d968e294073affff457b041c2be9868a40c1c71f4a35fcc1e45e5493067b/pytest_cov-7.1.0-py3-none-any.whl", hash = "sha256:a0461110b7865f9a271aa1b51e516c9a95de9d696734a2f71e3e78f46e1d4678", size = 22876, upload-time = "2026-03-21T20:11:14.438Z" }, +] + [[package]] name = "python-dateutil" version = "2.9.0.post0" From fcdac8c46a82da03139f5c3a10f5305a1ac58c9c Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Wed, 17 Jun 2026 18:11:21 +0300 Subject: [PATCH 19/27] Add missing unit test coverage In fact, went all the way and achieved 100% coverage in the files mainly touched by this PR. Lint and format edits too. --- otava/analysis.py | 7 +- otava/change_point_divisive/base.py | 41 ++- .../significance_test.py | 4 +- tests/analysis_test.py | 16 + tests/change_point_classes_test.py | 282 +++++++++++++++++- tests/change_point_divisive_test.py | 81 ++++- tests/report_test.py | 5 +- 7 files changed, 404 insertions(+), 32 deletions(-) diff --git a/otava/analysis.py b/otava/analysis.py index 518d66c7..0a51ebf9 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -48,13 +48,12 @@ class TTestStats(BaseStats): def copy(self): # replace() preserves the subclass; deep-copy the array so the copy is independent - return replace(self, permuted_qhats=self.permuted_qhats.copy()) + return replace(self) def to_json(self): obj = super().to_json() - obj["permuted_qhats"] = self.permuted_qhats.tolist() - obj["extreme_qhat_perm"] = self.extreme_qhat_perm - obj["n_perm"] = self.n_perm + obj["tstatistic"] = self.tstatistic + obj["degrees_of_freedom"] = self.degrees_of_freedom return obj diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 2e8f626c..aaa3419e 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -253,6 +253,7 @@ def to_json(self, rounded=True): "metric": self.metric, "index": int(self.index), "forward_change_percent": f"{self.forward_change_percent():.0f}", + "backward_change_percent": f"{self.backward_change_percent():.0f}", "magnitude": f"{self.magnitude():-0f}", "mean_before": f"{self.mean_before():-0f}", "stddev_before": f"{self.stddev_before():-0f}", @@ -472,6 +473,15 @@ def __len__(self): def __getitem__(self, n): return self._change_points[n] + def __contains__(self, metric): + """ + True if this container holds at least one ChangePoint where metric==metric + + This makes more sense, and is more efficient, for ChangePointsByMetric class, but we provide + the same functionality here for consistency. + """ + return metric in self.metrics() + def metrics(self) -> set: all_metrics = set() for row in self._change_points: @@ -593,16 +603,34 @@ def append(self, cpg: ChangePointGroup): for metric in cpg.metrics(): if metric not in self._change_points: self._change_points[metric] = [] - self._change_points[metric].append(cpg.select_metrics(metric)) + for metric1, cp in cpg.changes.items(): + if metric1 != cp.metric: + raise ValueError(f"metric field is not internally consistent. {metric1} != {cp.metric} at {cpg.time}") + if (not self._change_points[metric]) or cpg.time > self._change_points[metric][-1].time: + self._change_points[metric].append(cpg.select_metrics(metric)) + else: + raise ValueError( + "ChangePoints.extend() can only be used such that time is monotonically increasing" + ) def extend(self, cps: list[ChangePointGroup]): errmsg = "ChangePoints.extend() takes as argument a list of ChangePointGroup objects." if not isinstance(cps, list): raise TypeError(errmsg) - for obj in cps: - if not isinstance(obj, ChangePointGroup): + for cpg in cps: + if not isinstance(cpg, ChangePointGroup): raise TypeError(errmsg) - self.append(obj) + for metric1, cp in cpg.changes.items(): + if metric1 != cp.metric: + raise ValueError(f"metric field is not internally consistent. {metric1} != {cp.metric} at {cpg.time}") + if metric1 not in self._change_points: + self._change_points[metric1] = [] + if (not self._change_points[metric1]) or cpg.time > self._change_points[metric1][-1].time: + self._change_points[metric1].append(cpg) + else: + raise ValueError( + "ChangePoints.extend() can only be used such that time is monotonically increasing" + ) def by_time(self): return self.pivot() @@ -613,6 +641,9 @@ def by_metric(self): def items(self): return self._change_points.items() + def keys(self): + return self._change_points.keys() + def __iter__(self): return self.pivot().__iter__() @@ -631,7 +662,7 @@ def __getitem__(self, n): def __setitem__(self, metric: str, cpglist: ChangePointGroup): self._change_points[metric] = cpglist - def __in__(self, metric): + def __contains__(self, metric): return metric in self._change_points def metrics(self): diff --git a/otava/change_point_divisive/significance_test.py b/otava/change_point_divisive/significance_test.py index dcbddd42..1934ca3d 100644 --- a/otava/change_point_divisive/significance_test.py +++ b/otava/change_point_divisive/significance_test.py @@ -39,11 +39,11 @@ class PermutationStats(BaseStats): def copy(self): # replace() preserves the subclass; deep-copy the array so the copy is independent - return replace(self, permuted_qhats=self.permuted_qhats.copy()) + return replace(self, permuted_qhats=self.permuted_qhats) def to_json(self): obj = super().to_json() - obj["permuted_qhats"] = self.permuted_qhats.tolist() + obj["permuted_qhats"] = self.permuted_qhats obj["extreme_qhat_perm"] = self.extreme_qhat_perm obj["n_perm"] = self.n_perm return obj diff --git a/tests/analysis_test.py b/tests/analysis_test.py index 5d02716e..ef548a77 100644 --- a/tests/analysis_test.py +++ b/tests/analysis_test.py @@ -65,6 +65,22 @@ def test_single_series(): indexes = [c.index for c in cps] assert indexes == [10] + # incremental change point detection: + cps, _ = compute_change_points(series, max_pvalue=0.0001, new_data=0.47, old_weak_cp=cps) + indexes = [c.index for c in cps] + assert indexes == [10] + + # decrease window_len to generate more cp and hit the last two lines in code cov... + cps, _ = compute_change_points(series+[0.47, 0.48, 0.45, 0.01, 0.1, 0.22], max_pvalue=0.0001, new_data=0.27, old_weak_cp=cps, window_len=5) + indexes = [c.index for c in cps] + assert indexes == [10, 23] + + cps = [cps[0]] + cps[0].index = 2 + cps, _ = compute_change_points(series+[0.47, 0.48, 0.45, 0.01, 0.1, 0.22], max_pvalue=0.0001, new_data=0.27, old_weak_cp=cps, window_len=5) + indexes = [c.index for c in cps] + assert indexes == [10, 23] + def test_single_series_original(): series = [ diff --git a/tests/change_point_classes_test.py b/tests/change_point_classes_test.py index 82a52157..c17118a5 100644 --- a/tests/change_point_classes_test.py +++ b/tests/change_point_classes_test.py @@ -42,9 +42,7 @@ ) -# --------------------------------------------------------------------------- # # Helpers -# --------------------------------------------------------------------------- # def make_stats(left=(1.0, 1.0), right=(5.0, 5.0), pvalue=0.01): return BaseStats.calculate(list(left), list(right), pvalue) @@ -61,9 +59,7 @@ def make_group(time, metric="m", index=3, commit="sha"): ) -# --------------------------------------------------------------------------- # # BaseStats -# --------------------------------------------------------------------------- # def test_basestats_calculate_means_and_std(): s = BaseStats.calculate([10.0, 10.0, 10.0], [20.0, 20.0, 20.0], 0.02) assert s.mean_1 == 10.0 @@ -141,9 +137,7 @@ def test_basestats_copy_is_independent_and_keeps_class(): assert s.mean_1 != 999.0 -# --------------------------------------------------------------------------- # # CandidateChangePoint / ChangePoint / ChangePointSerializer -# --------------------------------------------------------------------------- # def test_changepoint_from_and_to_candidate(): candidate = CandidateChangePoint(index=7, qhat=2.5) stats = make_stats() @@ -208,9 +202,7 @@ def test_changepoint_to_json_delegates_to_serializer(): assert cp.to_json(rounded=False) == ChangePointSerializer(cp).to_json(rounded=False) -# --------------------------------------------------------------------------- # # ChangePointGroup -# --------------------------------------------------------------------------- # def test_group_getitem_metrics_and_iter(): g = ChangePointGroup( time=1.0, @@ -270,9 +262,7 @@ def test_group_copy_is_deep(): assert g.changes["m"].stats.mean_1 != -1.0 -# --------------------------------------------------------------------------- # # ChangePointsByTime -# --------------------------------------------------------------------------- # def test_bytime_append_keeps_time_order(): c = ChangePointsByTime() c.append(make_group(1.0, metric="a")) @@ -319,6 +309,11 @@ def test_bytime_constructor_rejects_dict_and_non_group(): ChangePointsByTime.from_list([object()]) +def test_bytime_base(): + with pytest.raises(TypeError): + ChangePointsByTime.from_list([object()]) + + def test_bytime_extend(): c = ChangePointsByTime() c.extend([make_group(1.0, "a"), make_group(2.0, "b")]) @@ -349,6 +344,35 @@ def test_bytime_at_commit(): with pytest.raises(LookupError): c.at_commit("nope") + with pytest.raises(TypeError): + c.append("xxx") + + with pytest.raises(TypeError): + c.extend("xxx") + + with pytest.raises(TypeError): + c.extend(["a", "b"]) + + +def test_bytime_append_out_of_order(): + c = ChangePointsByTime.from_list( + [make_group(1.0, "a", commit="c1"), make_group(2.0, "a", commit="c2")] + ) + assert c.at_commit("c2").time == 2.0 + with pytest.raises(TypeError): + c.extend("nope") + with pytest.raises(ValueError): + c.extend([make_group(1.0, "a", commit="c1")]) + + with pytest.raises(TypeError): + c.append("xxx") + + with pytest.raises(TypeError): + c.extend("xxx") + + with pytest.raises(TypeError): + c.extend(["a", "b"]) + def test_bytime_get_change_points_for_metric_sparse(): # 'a' appears at t=1 and t=3, 'b' only at t=2 -> sparse columns @@ -366,6 +390,9 @@ def test_bytime_pivot_and_items(): assert isinstance(pivoted, ChangePointsByMetric) assert pivoted.metrics() == {"a", "b"} assert {k for k, _ in c.items()} == {"a", "b"} + c = ChangePointsByTime.from_list( + [make_group(1.0, "a", commit="c1"), make_group(2.0, "b", commit="c2")] + ) def test_bytime_copy_is_deep(): @@ -377,9 +404,7 @@ def test_bytime_copy_is_deep(): assert c[0].changes["a"].stats.mean_1 != -1.0 -# --------------------------------------------------------------------------- # # ChangePointsByMetric -# --------------------------------------------------------------------------- # def test_bymetric_dict_constructor_sorts_by_time(): g1, g2 = make_group(1.0, "m"), make_group(2.0, "m") bm = ChangePointsByMetric.from_dict({"m": [g2, g1]}) # unsorted input @@ -392,6 +417,14 @@ def test_bymetric_dict_constructor_rejects_non_group(): ChangePointsByMetric.from_dict({"m": [object()]}) +def test_bymetric_dict_constructor_rejects_random(): + with pytest.raises(TypeError): + ChangePointsByMetric.from_dict([{}, {}]) + with pytest.raises(ValueError): + g1, g2 = make_group(1.0, "m"), make_group(2.0, "m") + _ = ChangePointsByMetric.from_dict({"XXX": [g1, g2]}) # unsorted input + + def test_from_dict_is_metric_keyed_and_returns_by_metric(): # A dict is inherently keyed by metric, so from_dict() always builds a # ChangePointsByMetric -- even when called on the base ChangePoints class. @@ -416,6 +449,8 @@ def test_bymetric_append_and_len(): assert bm.metrics() == {"a", "b"} # len == longest column assert len(bm) == 2 + with pytest.raises(TypeError): + bm.append("xxx") def test_bymetric_select_metrics(): @@ -440,6 +475,56 @@ def test_bymetric_by_time_roundtrip(): assert [g.time for g in again] == [1.0, 2.0] +def test_by_time_by_time_self(): + by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + same = by_time.by_time() + # Yes it's the same object, a no-op, not a copy. (Open to other opinions here) + assert by_time == same + assert by_time.at_timestamp(1.0) == same.at_timestamp(1.0) + + +def test_by_time_bm(): + by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + same = by_time.by_metric() + # Yes it's the same object, a no-op, not a copy. (Open to other opinions here) + assert by_time != same + assert by_time.at_timestamp(1.0) == same.at_timestamp(1.0) + + +def test_justcp_bm(): + by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + same = by_time.by_metric() + # Yes it's the same object, a no-op, not a copy. (Open to other opinions here) + assert by_time != same + assert by_time.at_timestamp(1.0) == same.at_timestamp(1.0) + + +def test_bm_bm_self(): + by_metric = ChangePointsByMetric.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + same = by_metric.by_metric() + assert by_metric == same + assert by_metric.metrics() == same.metrics() + + +def test_by_time_pivot_roundtrip(): + by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + bm = by_time.pivot() + again = bm.pivot() + bm_again = again.pivot() + assert isinstance(bm, ChangePointsByMetric) + assert isinstance(again, ChangePointsByTime) + assert isinstance(bm_again, ChangePointsByMetric) + assert [g.time for g in again] == [1.0, 2.0] + + +def test_by_time_self(): + by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) + same = by_time.by_time() + # Yes it's the same object, a no-op, not a copy. (Open to other opinions here) + assert by_time == same + assert by_time.at_timestamp(1.0) == same.at_timestamp(1.0) + + def test_bymetric_at_timestamp_and_at_commit(): bm = ChangePointsByMetric.from_list( [make_group(1.0, "a", commit="c1"), make_group(2.0, "b", commit="c2")] @@ -465,9 +550,167 @@ def test_bymetric__setitem_in(): assert bm._change_points["x"].changes["x"].index == 55 -# --------------------------------------------------------------------------- # +def test_bymetric_at_commit(): + c = ChangePointsByMetric.from_list( + [make_group(1.0, "a", commit="c1"), make_group(2.0, "a", commit="c2")] + ) + assert c.at_commit("c2").time == 2.0 + with pytest.raises(LookupError): + c.at_commit("nope") + + with pytest.raises(TypeError): + c.append("xxx") + + with pytest.raises(TypeError): + c.extend(["a", "b"]) + + +def test_bymetric_append_out_of_order(): + c = ChangePointsByMetric.from_list( + [make_group(1.0, "a", commit="c1"), make_group(2.0, "a", commit="c2")] + ) + with pytest.raises(TypeError): + c.extend("nope") + with pytest.raises(ValueError): + c.extend([make_group(1.1, "a", commit="c11")]) + + with pytest.raises(TypeError): + c.append("xxx") + + with pytest.raises(TypeError): + c.extend("xxx") + + with pytest.raises(TypeError): + c.extend(["a", "b"]) + + +def test_cpbm_getitem(): + _ = ChangePointsByMetric.from_list( + [make_group(1.0, "a", commit="c1"), make_group(2.0, "a", commit="c2")] + ) + + +def test_cpbm_append(): + cpbm = ChangePointsByMetric() + cpg = make_group(3.0, "b") + cpbm.append(cpg) + + cpg = make_group(1.0, "a") + cpbm.append(cpg) + + cpg = make_group(2.0, "a") + cpbm.append(cpg) + + with pytest.raises(ValueError): + cpg = make_group(1.1, "a") + cpbm.append(cpg) + + with pytest.raises(ValueError): + cpg = make_group(3.0, "b") + cpg.changes["b"].metric = "boo" + cpbm.append(cpg) + + +def test_cpbm_extend(): + cpbm = ChangePointsByMetric() + cpg = make_group(3.0, "b") + cpbm.extend([cpg]) + + cpg = make_group(1.0, "a") + cpbm.extend([cpg]) + + cpg = make_group(2.0, "a") + cpbm.extend([cpg]) + + with pytest.raises(ValueError): + cpg = make_group(1.1, "a") + cpbm.extend([cpg]) + + with pytest.raises(ValueError): + cpg = make_group(3.0, "b") + cpg.changes["b"].metric = "boo" + cpbm.extend([cpg]) + + +def test_contains(): + cpbm = ChangePointsByMetric() + cpg = make_group(1.0, "a") + cpbm.extend([cpg]) + assert "a" in cpbm._change_points + assert "b" not in cpbm._change_points + assert "a" in cpbm + assert "b" not in cpbm + assert "a" in cpbm.keys() + assert "b" not in cpbm.keys() + assert "a" in cpbm.metrics() + assert "b" not in cpbm.metrics() + + cpbt = ChangePoints() + cpg = make_group(9.0, "x") + cpbt.extend([cpg]) + assert "x" in cpbt + assert "y" not in cpbt + assert "x" in cpbt.metrics() + assert "y" not in cpbt.metrics() + + +def test_len(): + cpbm = ChangePointsByMetric.from_list([make_group(1, "a"), make_group(2, "a")]) + assert len(cpbm) == 2 + + +def test_inconsistent_metric(): + cpbm = ChangePointsByMetric() + cpg = make_group(1.0, "a") + cpbm.append(cpg) + cpg = make_group(2.0, "a") + cpbm.extend([cpg]) + with pytest.raises(ValueError): + cpg = make_group(3.0, "a") + cpg.changes["a"].metric = "foo" + cpbm.append(cpg) + with pytest.raises(ValueError): + cpg = make_group(4.0, "a") + cpg.changes["a"].metric = "foo" + cpbm.extend([cpg]) + with pytest.raises(ValueError): + cpbm._change_points["a"][0].changes["a"].metric = "foo" + _ = cpbm.get_change_points_for_metric("a") + + +def test_inconsistent_metric_by_time(): + cpbt = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "a")]) + with pytest.raises(ValueError): + cpbt._change_points[0].changes["a"].metric = "foo" + _ = cpbt.get_change_points_for_metric("a") + + +def test_select_metrics(): + cpbm = ChangePointsByMetric.from_list([make_group(1, "a"), make_group(2, "b")]) + + metrics = cpbm.select_metrics("a") + assert "a" in metrics + assert "b" not in metrics + + metrics = cpbm.select_metrics("b") + assert "b" in metrics + assert "a" not in metrics + + metrics = cpbm.select_metrics(["a", "b"]) + assert "a" in metrics + assert "b" in metrics + + with pytest.raises(KeyError): + metrics = cpbm.select_metrics(["a", "b", "c"]) + + with pytest.raises(KeyError): + metrics = cpbm.select_metrics("c") + + with pytest.raises(TypeError): + metrics = cpbm.select_metrics({}) + + # SignificanceTester helpers -# --------------------------------------------------------------------------- # def test_tester_compare_returns_basestats(): tester = SignificanceTester(0.05) stats = tester.compare([1.0, 1.0], [5.0, 5.0], 0.01) @@ -508,15 +751,22 @@ def test_tester_get_sides_merge_step(): assert list(right) == [3.0, 4.0, 5.0] +def test_tester_get_sides_not_exist(): + tester = SignificanceTester(0.05) + series = np.arange(9, dtype=float) + intervals = [slice(0, 3), slice(3, 6), slice(6, None)] + # candidate index matches the stop of the first interval -> merge step + with pytest.raises(ValueError): + left, right = tester.get_sides(CandidateChangePoint(index=-1, qhat=1.0), series, intervals) + + def test_tester_is_significant(): tester = SignificanceTester(0.05) assert tester.is_significant(make_cp(pvalue=0.01)) is True assert tester.is_significant(make_cp(pvalue=0.5)) is False -# --------------------------------------------------------------------------- # # Base ChangePoints behaviours shared by both representations -# --------------------------------------------------------------------------- # def test_base_changepoints_empty(): c = ChangePoints() assert len(c) == 0 diff --git a/tests/change_point_divisive_test.py b/tests/change_point_divisive_test.py index 65b31bed..ccdd9f67 100644 --- a/tests/change_point_divisive_test.py +++ b/tests/change_point_divisive_test.py @@ -22,7 +22,10 @@ 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 +from otava.change_point_divisive.significance_test import ( + PermutationsSignificanceTester, + PermutationStats, +) SEQUENCE = np.array([ 0.3, 2.4, 1.5, -0.9, -0.5, @@ -109,6 +112,15 @@ def test_permutation_test(): assert [cp.index for cp in cps] == CHANGE_POINTS_INDS +def test_permutation_test_integers(): + seed = 1 + sequence = np.array([0, 2, 1, -0, -0, 99, 98, 99, 149, 149, 149, 149, 148, 150]) + st = PermutationsSignificanceTester(max_pvalue=0.01, permutations=100, calculator=PairDistanceCalculator, seed=seed) + 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_ttest(): sequence = SEQUENCE.copy() st = TTestSignificanceTester(max_pvalue=0.01) @@ -120,7 +132,7 @@ def test_ttest(): 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) + stats = TTestStats(mean_1=1.0, mean_2=2.0, std_1=0.1, std_2=0.1, pvalue=0.001, tstatistic=0.1, degrees_of_freedom=18) # Sorted change points should work sorted_cps = [ @@ -143,3 +155,68 @@ def test_get_intervals_requires_sorted_change_points(): ] with pytest.raises(ValueError, match="Change points must be sorted by index"): tester.get_intervals(unsorted_cps) + + +def test_copy_ttest_stats(): + stats = TTestStats(mean_1=1.0, mean_2=2.0, std_1=0.1, std_2=0.1, pvalue=0.001, tstatistic=0.1, degrees_of_freedom=18) + stats2 = stats.copy() + stats2.mean_1 = 5.5 + stats.std_2 = 6.7 + stats2.tstatistic = 0.2 + + assert stats.mean_1 == 1.0 + assert stats.mean_2 == 2.0 + assert stats2.mean_1 == 5.5 + assert stats2.mean_2 == 2.0 + assert stats.std_2 == 6.7 + assert stats2.std_2 == 0.1 + assert stats.tstatistic == 0.1 + assert stats2.tstatistic == 0.2 + + +def test_ttest_stats_tojson(): + stats = TTestStats(mean_1=1.0, mean_2=2.0, std_1=0.1, std_2=0.1, pvalue=0.001, tstatistic=0.1, degrees_of_freedom=18) + obj = stats.to_json() + assert obj["degrees_of_freedom"] == 18 + + +def test_copy_permutation_stats(): + pvalue = np.float64(0.05) + mean_1 = np.float64(0.125) + mean_2 = np.float64(132.5) + std_1 = np.float64(1.2) + std_2 = np.float64(23.724285624546457) + permuted_qhats = np.array([202.13]), + extreme_qhat_perm = np.int64(100) + n_perm = 1 + + stats = PermutationStats(mean_1=mean_1, mean_2=mean_2, std_1=std_1, std_2=std_2, pvalue=pvalue, permuted_qhats=permuted_qhats, extreme_qhat_perm=extreme_qhat_perm, n_perm=n_perm) + stats2 = stats.copy() + + stats2.mean_1 = 5.5 + stats.std_2 = 6.7 + stats2.extreme_qhat_perm = 101 + + assert stats.mean_1 == 0.125 + assert stats.mean_2 == 132.5 + assert stats2.mean_1 == 5.5 + assert stats.std_2 == 6.7 + assert stats2.std_2 == 23.724285624546457 + assert stats.extreme_qhat_perm == 100 + assert stats2.extreme_qhat_perm == 101 + + +def test_permutation_stats_tojson(): + pvalue = np.float64(0.05) + mean_1 = np.float64(0.123) + mean_2 = np.float64(132.5) + std_1 = np.float64(1.2) + std_2 = np.float64(23.724285624546457) + permuted_qhats = np.array([202.13]), + extreme_qhat_perm = np.int64(100) + n_perm = 1 + + stats = PermutationStats(mean_1=mean_1, mean_2=mean_2, std_1=std_1, std_2=std_2, pvalue=pvalue, permuted_qhats=permuted_qhats, extreme_qhat_perm=extreme_qhat_perm, n_perm=n_perm) + + obj = stats.to_json() + assert obj["n_perm"] == 1 diff --git a/tests/report_test.py b/tests/report_test.py index 6e8b4836..14e0a286 100644 --- a/tests/report_test.py +++ b/tests/report_test.py @@ -79,6 +79,7 @@ def test_json_report(report): obj = json.loads(output) expected = {'test_name_from_config': [{'attributes': {}, 'changes': [{'forward_change_percent': '-11', + 'backward_change_percent': '12', 'index': 4, 'magnitude': '0.124108', 'mean_after': '1.801429', @@ -90,6 +91,7 @@ def test_json_report(report): 'time': 4}, {'attributes': {}, 'changes': [{'forward_change_percent': '-49', + 'backward_change_percent': '98', 'index': 6, 'magnitude': '0.977513', 'mean_after': '0.504000', @@ -100,7 +102,4 @@ def test_json_report(report): 'stddev_before': '0.067495'}], 'time': 6}]} assert isinstance(obj, dict) - from pprint import pprint - pprint(obj) - pprint(expected) assert obj == expected From 221cccd3d7e71c54638d8a2acf44725d82445140 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 29 Jun 2026 19:55:56 +0300 Subject: [PATCH 20/27] review: Remove __eq__() for ChangePoint class --- otava/analysis.py | 15 +++++++++++---- otava/change_point_divisive/base.py | 6 +----- tests/change_point_classes_test.py | 7 +++++-- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/otava/analysis.py b/otava/analysis.py index 0a51ebf9..21bb33c1 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -47,7 +47,7 @@ class TTestStats(BaseStats): degrees_of_freedom: int = 0 def copy(self): - # replace() preserves the subclass; deep-copy the array so the copy is independent + # replace() preserves the subclass return replace(self) def to_json(self): @@ -146,8 +146,13 @@ def merge( if weakest_cp.stats.change_magnitude() > min_magnitude: return change_points - # Remove the point from the list - weakest_cp_index = change_points.index(weakest_cp) + # Remove duplicate change points from the list (that is, at same index) + weakest_cp_index = None + for i, cp in enumerate(change_points): + if cp.index == weakest_cp.index: + weakest_cp_index = i + break + assert weakest_cp_index is not None del change_points[weakest_cp_index] # We can't continue yet, because by removing a change_point @@ -202,9 +207,11 @@ def split(series: Sequence[SupportsFloat], window_len: int = 30, max_pvalue: flo last_new_change_point_index = new_change_points[-1].index if new_change_points else 0 start = max(last_new_change_point_index, start + step) # incremental Otava can duplicate an old cp + cpindexes = [cp.index for cp in change_points] for cp in new_change_points: - if cp not in change_points: + if cp.index not in cpindexes: change_points += [cp] + cpindexes += [cp.index] # Sort change points by index; required by get_intervals() and maintained by merge() change_points.sort(key=lambda cp: cp.index) diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index aaa3419e..94693205 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -158,7 +158,7 @@ class ChangePoint(CandidateChangePoint, Generic[GenericStats]): """ ChangePoint class. - Defined by index and signigicance test statistic. + Defined by index and significance test statistic. This class is the basic change point that is used during computation and returned as a result. This class does not however carry additional attributes like metric, time, or commit sha. Those are in ChangePointGroup @@ -186,10 +186,6 @@ def copy(self): index=self.index, qhat=self.qhat, stats=self.stats.copy(), metric=self.metric ) - def __eq__(self, other): - """Helpful to identify new Change Points during divisive algorithm""" - return isinstance(other, self.__class__) and self.index == other.index - @classmethod def from_candidate( cls, candidate: CandidateChangePoint, stats: GenericStats diff --git a/tests/change_point_classes_test.py b/tests/change_point_classes_test.py index c17118a5..62a279a1 100644 --- a/tests/change_point_classes_test.py +++ b/tests/change_point_classes_test.py @@ -152,12 +152,15 @@ def test_changepoint_from_and_to_candidate(): assert back.qhat == 2.5 -def test_changepoint_equality_is_by_index(): +def test_changepoint_equality_is_NOT_JUST_by_index(): a = make_cp(index=3) b = make_cp(index=3, left=(2.0, 2.0)) # different stats, same index c = make_cp(index=4) - assert a == b + d = make_cp(index=4) + # assert a == b # Used to be true, but was not meaningful as general equality + assert a != b assert a != c + assert c == d def test_changepoint_metric_defaults_to_none(): From 5de29b30cfe454f8210fb11141bb195fcb84cbab Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 29 Jun 2026 19:57:17 +0300 Subject: [PATCH 21/27] review: PermutationStats.copy() wasn't copying NDArray deeply --- otava/change_point_divisive/significance_test.py | 6 ++++-- tests/change_point_divisive_test.py | 12 ++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/otava/change_point_divisive/significance_test.py b/otava/change_point_divisive/significance_test.py index 1934ca3d..92914847 100644 --- a/otava/change_point_divisive/significance_test.py +++ b/otava/change_point_divisive/significance_test.py @@ -19,6 +19,7 @@ from typing import List, Optional, Type import numpy as np + from numpy.typing import NDArray from otava.change_point_divisive.base import ( @@ -38,8 +39,9 @@ class PermutationStats(BaseStats): n_perm: int def copy(self): - # replace() preserves the subclass; deep-copy the array so the copy is independent - return replace(self, permuted_qhats=self.permuted_qhats) + c = replace(self) + c.permuted_qhats = self.permuted_qhats.copy() + return c def to_json(self): obj = super().to_json() diff --git a/tests/change_point_divisive_test.py b/tests/change_point_divisive_test.py index ccdd9f67..7c1aa226 100644 --- a/tests/change_point_divisive_test.py +++ b/tests/change_point_divisive_test.py @@ -186,16 +186,19 @@ def test_copy_permutation_stats(): mean_2 = np.float64(132.5) std_1 = np.float64(1.2) std_2 = np.float64(23.724285624546457) - permuted_qhats = np.array([202.13]), + permuted_qhats = np.array([[202.13, 99.0, 105.5],[1,2,3]]) extreme_qhat_perm = np.int64(100) n_perm = 1 stats = PermutationStats(mean_1=mean_1, mean_2=mean_2, std_1=std_1, std_2=std_2, pvalue=pvalue, permuted_qhats=permuted_qhats, extreme_qhat_perm=extreme_qhat_perm, n_perm=n_perm) + stats2 = stats.copy() stats2.mean_1 = 5.5 stats.std_2 = 6.7 stats2.extreme_qhat_perm = 101 + stats2.permuted_qhats[0][0] = 215.14 + stats.permuted_qhats[1][1] = 99.99 assert stats.mean_1 == 0.125 assert stats.mean_2 == 132.5 @@ -204,7 +207,12 @@ def test_copy_permutation_stats(): assert stats2.std_2 == 23.724285624546457 assert stats.extreme_qhat_perm == 100 assert stats2.extreme_qhat_perm == 101 - + assert stats.permuted_qhats[0][0] == 202.13 + assert stats.permuted_qhats[1][1] == 99.99 + assert stats2.permuted_qhats[0][0] == 215.14 + assert id(stats) != id(stats2) + assert id(stats.permuted_qhats) != id(stats2.permuted_qhats) + assert not stats.permuted_qhats is stats2.permuted_qhats def test_permutation_stats_tojson(): pvalue = np.float64(0.05) From 1cc15d82a75e3bfcf70ae6d3fd7406e2cf10984a Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 29 Jun 2026 21:38:27 +0300 Subject: [PATCH 22/27] review: Fix and add test for ReportType.REGRESSIONS_ONLY --- otava/bigquery.py | 4 ++-- otava/report.py | 5 +++-- tests/report_test.py | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/otava/bigquery.py b/otava/bigquery.py index 729ffdc7..8ceb5a5d 100644 --- a/otava/bigquery.py +++ b/otava/bigquery.py @@ -22,7 +22,7 @@ from google.cloud import bigquery from google.oauth2 import service_account -from otava.change_point_divisive.base import ChangePointGroup +from otava.change_point_divisive.base import ChangePointGroup, ChangePointSerializer from otava.test_config import BigQueryTestConfig @@ -89,7 +89,7 @@ def insert_change_point( attributes: Dict, change_point_group: ChangePointGroup, ): - change_point = change_point_group[metric_name] + change_point = ChangePointSerializer(change_point_group[metric_name]) kwargs = {**attributes, **{test.time_column: datetime.utcfromtimestamp(change_point_group.time)}} update_stmt = test.update_stmt.format( metric=metric_name, diff --git a/otava/report.py b/otava/report.py index 1244af07..5431b825 100644 --- a/otava/report.py +++ b/otava/report.py @@ -109,8 +109,9 @@ def __format_regressions_only(self, test_name: str) -> str: output = [] for cpg in self.__change_points: regressions = [] - for cp in cpg.changes: - metric = self.__series.metrics[cp.metric] + for metric_name, cp in cpg.changes.items(): + cp = ChangePointSerializer(cp) + metric = self.__series.metrics[metric_name] if metric.direction * cp.forward_change_percent() < 0: regressions.append( ( diff --git a/tests/report_test.py b/tests/report_test.py index 14e0a286..5391409f 100644 --- a/tests/report_test.py +++ b/tests/report_test.py @@ -103,3 +103,21 @@ def test_json_report(report): 'time': 6}]} assert isinstance(obj, dict) assert obj == expected + +def test_regression_only_report(report): + output = report.produce_report("test_name_for_regression_only", ReportType.REGRESSIONS_ONLY) + assert output + +def test_simplereport_regression_only(): + from otava.series import Metric, Series + from otava.report import Report, ReportType + s = Series('t', None, list(range(11)), + {'a': Metric(1, 1.0), 'b': Metric(1, 1.0)}, + {'a': [1.02,0.95,0.99,1.00,1.12,0.90,0.50,0.51,0.48,0.48,0.55], + 'b': [2.02,2.03,2.01,2.04,1.82,1.85,1.79,1.81,1.80,1.76,1.78]}, + {}) + out = Report(s, s.analyze().change_points_by_time).produce_report('t', ReportType.REGRESSIONS_ONLY) + print(out) + assert "Regressions" in out + assert "-11.0" in out + assert "-49.4" in out From beff89145d0b97c096f983d3f27924c293b22114 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 29 Jun 2026 21:52:40 +0300 Subject: [PATCH 23/27] review: datatype should be datetime --- otava/series.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otava/series.py b/otava/series.py index 372a6491..e93a4bee 100644 --- a/otava/series.py +++ b/otava/series.py @@ -139,7 +139,7 @@ class AnalyzedSeries: options: AnalysisOptions change_points: ChangePointsByMetric change_points_by_time: ChangePointsByTime - change_points_timestamp: ChangePointsByMetric + change_points_timestamp: datetime def __init__( self, series: Series, options: AnalysisOptions, change_points: Dict[str, ChangePoint] = None From 6946efb693e6b795d93c9a62cfb3caedb0df3848 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 29 Jun 2026 22:35:25 +0300 Subject: [PATCH 24/27] review: to_json(): fix the case with multiple metrics --- otava/series.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/otava/series.py b/otava/series.py index e93a4bee..293a1c32 100644 --- a/otava/series.py +++ b/otava/series.py @@ -359,12 +359,18 @@ def metric(self, name: str) -> Metric: def to_json(self): change_points_json = {} - for cps in self.change_points: - change_points_json = [cp.to_json(rounded=False) for cp in cps] + cpbm = self.change_points.by_metric() + for metric_name in self.change_points.metrics(): + change_points_json[metric_name] = [] + for cp in cpbm.select_metrics(metric_name): + change_points_json[metric_name].append(cp.to_json(rounded=False)) weak_change_points_json = {} - for cps in self.weak_change_points: - weak_change_points_json = [cp.to_json(rounded=False) for cp in cps] + wcpbm = self.change_points.by_metric() + for metric_name in self.change_points.metrics(): + weak_change_points_json[metric_name] = [] + for cp in wcpbm.select_metrics(metric_name): + weak_change_points_json[metric_name].append(cp.to_json(rounded=False)) data_json = {} for metric, datapoints in self.__series.data.items(): From ab8347fb360660a9af91a5b6716aca6d8ed9d35d Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 29 Jun 2026 23:00:10 +0300 Subject: [PATCH 25/27] review: Tests cleanup and fixes --- tests/change_point_classes_test.py | 22 ---------------------- tests/report_test.py | 7 ------- 2 files changed, 29 deletions(-) diff --git a/tests/change_point_classes_test.py b/tests/change_point_classes_test.py index 62a279a1..8bdce3ff 100644 --- a/tests/change_point_classes_test.py +++ b/tests/change_point_classes_test.py @@ -478,14 +478,6 @@ def test_bymetric_by_time_roundtrip(): assert [g.time for g in again] == [1.0, 2.0] -def test_by_time_by_time_self(): - by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) - same = by_time.by_time() - # Yes it's the same object, a no-op, not a copy. (Open to other opinions here) - assert by_time == same - assert by_time.at_timestamp(1.0) == same.at_timestamp(1.0) - - def test_by_time_bm(): by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) same = by_time.by_metric() @@ -494,14 +486,6 @@ def test_by_time_bm(): assert by_time.at_timestamp(1.0) == same.at_timestamp(1.0) -def test_justcp_bm(): - by_time = ChangePointsByTime.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) - same = by_time.by_metric() - # Yes it's the same object, a no-op, not a copy. (Open to other opinions here) - assert by_time != same - assert by_time.at_timestamp(1.0) == same.at_timestamp(1.0) - - def test_bm_bm_self(): by_metric = ChangePointsByMetric.from_list([make_group(1.0, "a"), make_group(2.0, "b")]) same = by_metric.by_metric() @@ -587,12 +571,6 @@ def test_bymetric_append_out_of_order(): c.extend(["a", "b"]) -def test_cpbm_getitem(): - _ = ChangePointsByMetric.from_list( - [make_group(1.0, "a", commit="c1"), make_group(2.0, "a", commit="c2")] - ) - - def test_cpbm_append(): cpbm = ChangePointsByMetric() cpg = make_group(3.0, "b") diff --git a/tests/report_test.py b/tests/report_test.py index 5391409f..a72bd67d 100644 --- a/tests/report_test.py +++ b/tests/report_test.py @@ -40,13 +40,6 @@ def series(): @pytest.fixture(scope="module") def change_points(series): - # o = AnalysisOptions() - # print("default AnalysisOptions are...") - # print(o) - # o.max_pvalue = 0.001 - # print("setting them to") - # print(o) - # return series.analyze(options=o).change_points_by_time return series.analyze().change_points_by_time From c4e9cc91f062fbe9a52d5a89c17250f283204222 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 29 Jun 2026 23:11:30 +0300 Subject: [PATCH 26/27] lint & format (tox) --- otava/change_point_divisive/significance_test.py | 1 - tests/change_point_divisive_test.py | 5 +++-- tests/report_test.py | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/otava/change_point_divisive/significance_test.py b/otava/change_point_divisive/significance_test.py index 92914847..61ba0635 100644 --- a/otava/change_point_divisive/significance_test.py +++ b/otava/change_point_divisive/significance_test.py @@ -19,7 +19,6 @@ from typing import List, Optional, Type import numpy as np - from numpy.typing import NDArray from otava.change_point_divisive.base import ( diff --git a/tests/change_point_divisive_test.py b/tests/change_point_divisive_test.py index 7c1aa226..1acc623f 100644 --- a/tests/change_point_divisive_test.py +++ b/tests/change_point_divisive_test.py @@ -186,7 +186,7 @@ def test_copy_permutation_stats(): mean_2 = np.float64(132.5) std_1 = np.float64(1.2) std_2 = np.float64(23.724285624546457) - permuted_qhats = np.array([[202.13, 99.0, 105.5],[1,2,3]]) + permuted_qhats = np.array([[202.13, 99.0, 105.5], [1, 2, 3]]) extreme_qhat_perm = np.int64(100) n_perm = 1 @@ -212,7 +212,8 @@ def test_copy_permutation_stats(): assert stats2.permuted_qhats[0][0] == 215.14 assert id(stats) != id(stats2) assert id(stats.permuted_qhats) != id(stats2.permuted_qhats) - assert not stats.permuted_qhats is stats2.permuted_qhats + assert stats.permuted_qhats is not stats2.permuted_qhats + def test_permutation_stats_tojson(): pvalue = np.float64(0.05) diff --git a/tests/report_test.py b/tests/report_test.py index a72bd67d..31263b0b 100644 --- a/tests/report_test.py +++ b/tests/report_test.py @@ -97,18 +97,18 @@ def test_json_report(report): assert isinstance(obj, dict) assert obj == expected + def test_regression_only_report(report): output = report.produce_report("test_name_for_regression_only", ReportType.REGRESSIONS_ONLY) assert output + def test_simplereport_regression_only(): - from otava.series import Metric, Series - from otava.report import Report, ReportType s = Series('t', None, list(range(11)), - {'a': Metric(1, 1.0), 'b': Metric(1, 1.0)}, - {'a': [1.02,0.95,0.99,1.00,1.12,0.90,0.50,0.51,0.48,0.48,0.55], - 'b': [2.02,2.03,2.01,2.04,1.82,1.85,1.79,1.81,1.80,1.76,1.78]}, - {}) + {'a': Metric(1, 1.0), 'b': Metric(1, 1.0)}, + {'a': [1.02, 0.95, 0.99, 1.00, 1.12, 0.90, 0.50, 0.51, 0.48, 0.48, 0.55], + 'b': [2.02, 2.03, 2.01, 2.04, 1.82, 1.85, 1.79, 1.81, 1.80, 1.76, 1.78]}, + {}) out = Report(s, s.analyze().change_points_by_time).produce_report('t', ReportType.REGRESSIONS_ONLY) print(out) assert "Regressions" in out From ff65e35226c620582df8f6b6f14b408d31c4ed26 Mon Sep 17 00:00:00 2001 From: Henrik Ingo Date: Mon, 29 Jun 2026 23:25:36 +0300 Subject: [PATCH 27/27] review: Add comment blocks to ChangePointsBy* classes --- otava/change_point_divisive/base.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/otava/change_point_divisive/base.py b/otava/change_point_divisive/base.py index 94693205..8f309616 100644 --- a/otava/change_point_divisive/base.py +++ b/otava/change_point_divisive/base.py @@ -538,6 +538,18 @@ def pivot(self): class ChangePointsByTime(ChangePoints): + """ + Implementation of ChangePoints class where the internal structure is ordered by time/commit. + + In fact, this is the default way, and therefore all of this class' implementation is already + in its parent class ChangePoints. However, you can still create instances from this class + to make it explicit that your code at that point explicitly wanted a collection of ChangePoints + ordered by time. + + The pivot() method will return a new object (a copy) holding the same data, but ordered by metrics + as the primary and optimized axis. The method by_metric() can be used for the same purpose. Note + that the method by_time() is a no-op and returns self, it doesn't even do a copy. + """ @classmethod def from_dict(cls, cps: dict): """ @@ -551,7 +563,14 @@ def from_dict(cls, cps: dict): class ChangePointsByMetric(ChangePoints): """ - Provides same interface as ChangePointsByTime, but internally stores with metric first. + Provides same interface as ChangePointsByTime, but internally stores with metric first. + + You can create empty instances of this class, or you can also use the factory method + `ChangePoints.from_dict()` to get an instance of this type. + + The pivot() method will return a new object (a copy) holding the same data, but ordered by time + as the primary and optimized axis. The method by_time() can be used for the same purpose. Note + that the method by_metric() is a no-op and returns self, it doesn't even do a copy. """ def __init__(self):