Copilot commented on code in PR #178:
URL: https://github.com/apache/otava/pull/178#discussion_r3910507672
##########
otava/serialization.py:
##########
@@ -15,58 +15,26 @@
# specific language governing permissions and limitations
# under the License.
-from datetime import datetime
-from typing import Dict, List, Optional
-
-from pydantic import BaseModel, ConfigDict
-
-JsonScalar = str | int | float | bool | None
-
-
-class AnalysisOptionsModel(BaseModel):
- model_config = ConfigDict(extra="forbid", validate_assignment=True)
-
- window_len: int = 50
- max_pvalue: float = 0.001
- min_magnitude: float = 0.0
- orig_edivisive: bool = False
-
-
-class MetricModel(BaseModel):
- direction: Optional[int] = None
- scale: Optional[float] = None
- unit: str = ""
-
-
-class ChangePointModel(BaseModel):
- metric: Optional[str] = None
- index: int
- qhat: float
- forward_change_percent: float
- magnitude: float
- mean_before: float
- stddev_before: float
- mean_after: float
- stddev_after: float
- pvalue: float
-
-
-class ChangePointGroupModel(BaseModel):
- time: int | float
- attributes: Dict[str, JsonScalar]
- changes: List[ChangePointModel]
-
-
-class AnalyzedSeriesModel(BaseModel):
- model_config = ConfigDict(arbitrary_types_allowed=False)
-
- test_name: str
- time: List[int | float]
- change_points_timestamp: datetime
- branch_name: Optional[str] = None
- options: AnalysisOptionsModel
- metrics: Dict[str, MetricModel]
- attributes: Dict[str, List[JsonScalar]]
- data: Dict[str, List[Optional[float]]]
- change_points: Dict[str, List[ChangePointGroupModel]]
- weak_change_points: Dict[str, List[ChangePointGroupModel]]
+"""Compatibility imports for the pre-#78 serialization model names.
+
+The domain models are now the persistence API. These aliases remain for one
+release so callers can migrate imports independently.
+"""
+
+from otava.change_point_divisive.base import ChangePoint, ChangePointGroup,
JsonScalar
+from otava.series import AnalysisOptions, AnalyzedSeries, Metric
+
+__all__ = [
+ "AnalysisOptionsModel",
+ "AnalyzedSeriesModel",
+ "ChangePointGroupModel",
+ "ChangePointModel",
+ "JsonScalar",
+ "MetricModel",
+]
+
+AnalysisOptionsModel = AnalysisOptions
+MetricModel = Metric
+ChangePointModel = ChangePoint
+ChangePointGroupModel = ChangePointGroup
Review Comment:
These aliases preserve import names but not the compatibility models'
validation contracts. The old `ChangePointModel` accepted a flat object
containing `mean_before`, `pvalue`, and related fields, whereas `ChangePoint`
requires nested `stats`; likewise, the old `ChangePointGroupModel` accepted
`changes` as a list, while `ChangePointGroup` requires a metric-keyed dict.
Existing callers can import the aliases but their previously valid payloads now
fail validation. Keep compatibility wrapper models/validators for the
documented transition release.
##########
otava/series.py:
##########
@@ -75,20 +78,42 @@ class Series:
def __init__(
self,
test_name: str,
- branch: Optional[str],
- time: List[int | float],
- metrics: Dict[str, Metric],
- data: Dict[str, List[float]],
- attributes: Dict[str, List[JsonScalar]],
+ branch: Optional[str] = None,
+ time: Optional[List[int | float]] = None,
+ metrics: Optional[Dict[str, Metric]] = None,
+ data: Optional[Dict[str, List[float]]] = None,
Review Comment:
Historical persistence documents allowed `null` data points
(`AnalyzedSeriesModel.data` was `List[Optional[float]]`), and the serializer
below still explicitly preserves `None`. Routing restoration through this new
Pydantic field now rejects those previously valid documents, contrary to the
compatibility goal. Keep the model and constructor annotations optional at the
element level.
This issue also appears in the following locations of the same file:
- line 98
- line 167
- line 267
- line 497
--
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]
