Gerrrr commented on code in PR #161:
URL: https://github.com/apache/otava/pull/161#discussion_r3488732967
##########
tests/series_test.py:
##########
@@ -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
Review Comment:
Should `assert cpjson` assert on the content of that variable?
##########
otava/report.py:
##########
@@ -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])
Review Comment:
```python
>>> 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]},
... {})
>>> Report(s, s.analyze().change_points_by_time).produce_report('t',
ReportType.REGRESSIONS_ONLY)
...
Traceback (most recent call last):
File "<python-input-0>", line 8, in <module>
Report(s, s.analyze().change_points_by_time).produce_report('t',
ReportType.REGRESSIONS_ONLY)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/asorokoumov/Projects/otava/otava/report.py", line 57, in
produce_report
return self.__format_regressions_only(test_name)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
File "/Users/asorokoumov/Projects/otava/otava/report.py", line 113, in
__format_regressions_only
metric = self.__series.metrics[cp.metric]
^^^^^^^^^
AttributeError: 'str' object has no attribute 'metric'
```
`LOG` and `JSON` work fine.
##########
tests/report_test.py:
##########
@@ -40,6 +40,13 @@ def series():
@pytest.fixture(scope="module")
def change_points(series):
+ # o = AnalysisOptions()
Review Comment:
nit: let's remove commented WIP code before merging this PR
##########
otava/bigquery.py:
##########
@@ -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]
Review Comment:
```python
>>> from otava.change_point_divisive.base import ChangePoint,
ChangePointGroup, BaseStats
>>> stats = BaseStats(pvalue=0.001, mean_1=1.0, mean_2=2.0, std_1=0.0,
std_2=0.0)
>>> cpg = ChangePointGroup(time=100.0, attributes={'commit':'sha'},
... changes={'m': ChangePoint(index=3, qhat=1.0,
stats=stats, metric='m')})
...
>>> cpg['m'].forward_change_percent()
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
cpg['m'].forward_change_percent()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'ChangePoint' object has no attribute
'forward_change_percent'
```
It should probably be:
```python
change_point = ChangePointSerializer(change_point_group[metric_name])
```
like in `postgres.py`
##########
otava/series.py:
##########
@@ -216,14 +137,17 @@ class AnalyzedSeries:
__series: Series
options: AnalysisOptions
- change_points: Dict[str, List[ChangePoint]]
- change_points_by_time: List[ChangePointGroup]
- change_points_timestamp: Any
+ change_points: ChangePointsByMetric
+ change_points_by_time: ChangePointsByTime
+ change_points_timestamp: ChangePointsByMetric
Review Comment:
Should this be `change_points_timestamp: ChangePointsByTime`?
##########
otava/change_point_divisive/base.py:
##########
@@ -40,39 +155,619 @@ class BaseStats:
@dataclass
class ChangePoint(CandidateChangePoint, Generic[GenericStats]):
- '''Change point class, defined by index and signigicance test statistic.'''
+ """
+ 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 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'''
+ """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) -> 'ChangePoint[GenericStats]':
+ def from_candidate(
+ cls, candidate: CandidateChangePoint, stats: GenericStats
+ ) -> "ChangePoint[GenericStats]":
return cls(
index=candidate.index,
qhat=candidate.qhat,
stats=stats,
)
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}",
+ "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}",
+ "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 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.
+ 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 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):
+ return self.attributes.get("commit")
+
+ def datetime(self):
+ return datetime.fromtimestamp(self.time, timezone.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 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 used.
+ """
+
+ 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[ChangePointGroup]):
+ """Build from a list of ChangePointGroup objects, ordered by time."""
+ if not isinstance(cps, list):
+ raise TypeError(
+ f"from_list() argument must be a list. Got {type(cps)}."
+ )
+ 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(
+ "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
+
+ def copy(self):
+ """
+ Copy constructor.
+
+ 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):
+ return self
+
+ def by_metric(self):
+ return self.pivot()
+
+ 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: 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):
+ 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 __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:
+ all_metrics.update(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 subset of the columns.
+
+ Note: The internal data structure doesn't do anything to make this
+ request efficient. This will loop over all ChangePointGroup s.
+ 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)
+ 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:
+ 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)
+ return by_metric
+
+
+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()?"
+ )
+
+
+class ChangePointsByMetric(ChangePoints):
+ """
+ Provides same interface as ChangePointsByTime, but internally stores with
metric first.
+ """
+
+ 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 = {}
+
+ # 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):
+ """
+ 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):
+ 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():
+ if metric not in self._change_points:
+ self._change_points[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 cpg in cps:
+ if not isinstance(cpg, ChangePointGroup):
+ raise TypeError(errmsg)
+ 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()
+
+ def by_metric(self):
+ return self
+
+ def items(self):
+ return self._change_points.items()
+
+ def keys(self):
+ return self._change_points.keys()
+
+ 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):
+ """
+ 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]
Review Comment:
Thinking out loud, we may want to cache both views. Python's `__get__` is
expected to work at O(1). Here, we do full rebuild, then return a single item.
##########
tests/change_point_classes_test.py:
##########
@@ -0,0 +1,773 @@
+# 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_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")])
+ 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")
+
+ 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
+ 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"}
+ c = ChangePointsByTime.from_list(
+ [make_group(1.0, "a", commit="c1"), make_group(2.0, "b", commit="c2")]
+ )
+
+
+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_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.
+ 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
+ with pytest.raises(TypeError):
+ bm.append("xxx")
+
+
+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_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)
+
Review Comment:
It would be cool to have test asserting semantical parity between `ByTime`
and `ByMetric`. Maybe property-based tests?
##########
tests/change_point_classes_test.py:
##########
@@ -0,0 +1,773 @@
+# 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_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")])
+ 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")
+
+ 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
+ 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"}
+ c = ChangePointsByTime.from_list(
+ [make_group(1.0, "a", commit="c1"), make_group(2.0, "b", commit="c2")]
+ )
+
+
+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_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.
+ 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
+ with pytest.raises(TypeError):
+ bm.append("xxx")
+
+
+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_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():
Review Comment:
This test is identical to `test_justcp_bm`
##########
otava/series.py:
##########
@@ -445,12 +365,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]
Review Comment:
```python
>>> from otava.series import Metric, Series
>>> 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]},
... {})
...
>>> j = s.analyze().to_json()
>>> print(type(j['change_points']))
<class 'list'>
>>> print(len(j['change_points']), j['change_points'][0]['metric'])
1 a
```
This is an incorrect answer because we lost `b`. At the very least, we
should do `change_points_json[metric] = ` instead of change_points_json = `.
While at it, let's take a step back evaluate the following scenario
Looking at `to_json`/`from_json` a bit more, I'd say we have a problem here.
Say, we have a new entry time with `time=100, commit=abc123` and a changepoint
that regressed 2 metrics - `latency` got +50% and throughput `-20%`. In memory,
this is a single `ChangePointGroup` with 2 changepoints.
If we apply the fix described above, it would look like:
```json
{
"latency": [{"metric": "latency", "index": 5,
"forward_change_percent": 50, ...}],
"throughput": [{"metric": "throughput", "index": 5,
"forward_change_percent": -20, ...}]
}
```
Note that `time` and `commit` attributes are missing. If we to write a
roundtrip test (we should!), it would fail here.
Wouldn't it be cool if `to_json` persisted the attributes, so we'd get:
```json
{
"time": 100.0,
"attributes": {"commit": "abc123"},
"changes": [
{"metric": "latency", "index": 5, "forward_change_percent": 50, ...},
{"metric": "throughput", "index": 5, "forward_change_percent": -20, ...}
]
}
```
##########
otava/change_point_divisive/significance_test.py:
##########
@@ -37,6 +37,17 @@ 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
Review Comment:
this comment is not true :)
```python
>> import numpy as np
>>> from otava.change_point_divisive.significance_test import
PermutationStats
>>> qhats = np.array([0.1, 0.2, 0.3])
>>> s = PermutationStats(pvalue=0.01, mean_1=1, mean_2=2, std_1=0, std_2=0,
... permuted_qhats=qhats, extreme_qhat_perm=1, n_perm=3)
>>> c = s.copy()
...
>>> print(c.permuted_qhats is s.permuted_qhats)
True # this should be have been false if that would be a deep-copy because
2 copies would have had different pointers.
>>> c.permuted_qhats[0] = -999.0
>>> print(s.permuted_qhats[0] == -999.0)
True
```
##########
tests/change_point_classes_test.py:
##########
@@ -0,0 +1,773 @@
+# 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_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")])
+ 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")
+
+ 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
+ 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"}
+ c = ChangePointsByTime.from_list(
+ [make_group(1.0, "a", commit="c1"), make_group(2.0, "b", commit="c2")]
+ )
+
+
+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_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.
+ 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
+ with pytest.raises(TypeError):
+ bm.append("xxx")
+
+
+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_by_time_by_time_self():
Review Comment:
This seems identical to a test on the line 520.
##########
otava/change_point_divisive/base.py:
##########
@@ -14,61 +14,640 @@
# 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, Sequence,
SupportsFloat, Any
+import numpy as np
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
+ 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)
@dataclass
class ChangePoint(CandidateChangePoint, Generic[GenericStats]):
- '''Change point class, defined by index and signigicance test statistic.'''
+ """
+ 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) -> 'ChangePoint[GenericStats]':
+ def from_candidate(
+ cls, candidate: CandidateChangePoint, stats: GenericStats
+ ) -> "ChangePoint[GenericStats]":
return cls(
index=candidate.index,
qhat=candidate.qhat,
stats=stats,
)
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
Review Comment:
Bump on this one. It would be awesome to get rid of it before merging this
PR or at least create a follow-up issue.
##########
tests/change_point_classes_test.py:
##########
@@ -0,0 +1,773 @@
+# 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_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")])
+ 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")
+
+ 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
+ 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"}
+ c = ChangePointsByTime.from_list(
+ [make_group(1.0, "a", commit="c1"), make_group(2.0, "b", commit="c2")]
+ )
+
+
+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_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.
+ 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
+ with pytest.raises(TypeError):
+ bm.append("xxx")
+
+
+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_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")]
+ )
+ 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
+
+
+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
+
+
+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(
Review Comment:
This looks like an unfinished/WIP test. We should either finish or remove it.
##########
otava/change_point_divisive/base.py:
##########
@@ -40,39 +155,619 @@ class BaseStats:
@dataclass
class ChangePoint(CandidateChangePoint, Generic[GenericStats]):
- '''Change point class, defined by index and signigicance test statistic.'''
+ """
+ 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 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'''
+ """Helpful to identify new Change Points during divisive algorithm"""
return isinstance(other, self.__class__) and self.index == other.index
Review Comment:
I am not sure comparing change points by index only is correct:
```python
>> from otava.change_point_divisive.base import ChangePoint, BaseStats
>>> stats = BaseStats(pvalue=0.001, mean_1=1.0, mean_2=2.0, std_1=0.1,
std_2=0.1)
>>> cp_latency = ChangePoint(index=5, qhat=1.0, stats=stats,
metric="latency")
>>> cp_throughput = ChangePoint(index=5, qhat=1.0, stats=stats,
metric="throughput")
>>> print(cp_latency == cp_throughput)
True
>>> all_cps = [cp_latency, cp_throughput]
>>> all_cps.remove(ChangePoint(index=5, qhat=0.0, stats=stats,
metric="throughput"))
>>> print(all_cps)
[ChangePoint(index=5, qhat=1.0, stats=BaseStats(pvalue=0.001, mean_1=1.0,
mean_2=2.0, std_1=0.1, std_2=0.1), metric='throughput')]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
