Copilot commented on code in PR #171:
URL: https://github.com/apache/otava/pull/171#discussion_r3809801824
##########
otava/report.py:
##########
@@ -103,7 +103,37 @@ def __format_log_annotated(self, test_name: str) -> str:
def __format_json(self, test_name: str) -> str:
import json
- return json.dumps({test_name: [cpg.to_json(rounded=True) for cpg in
self.__change_points]})
+ return json.dumps(
+ {
+ test_name: [
+ self.__format_change_point_group_json(cpg) for cpg in
self.__change_points
+ ]
+ }
+ )
+
+ @staticmethod
+ def __format_change_point_group_json(cpg):
+ return {
+ "time": cpg.time,
+ "attributes": cpg.attributes,
+ "changes": [Report.__format_change_point_json(cp) for cp in
cpg.changes.values()],
+ }
+
+ @staticmethod
+ def __format_change_point_json(cp):
+ cp = ChangePointSerializer(cp)
+ return {
+ "metric": cp.metric,
+ "index": int(cp.index),
+ "forward_change_percent": f"{cp.forward_change_percent():.0f}",
+ "backward_change_percent": f"{cp.backward_change_percent():.0f}",
+ "magnitude": f"{cp.magnitude():-0f}",
+ "mean_before": f"{cp.mean_before():-0f}",
+ "stddev_before": f"{cp.stddev_before():-0f}",
+ "mean_after": f"{cp.mean_after():-0f}",
+ "stddev_after": f"{cp.stddev_after():-0f}",
+ "pvalue": f"{cp.pvalue():-0f}",
+ }
Review Comment:
The JSON report formatter is using the format specifier `:-0f` for several
float fields (magnitude/means/stddev/pvalue). That format spec is
invalid/incorrect for the expected JSON output and will either raise at runtime
or round everything to 0 decimals (breaking
`tests/report_test.py::test_json_report`, which expects 6-decimal strings like
`"0.124108"`).
##########
otava/series.py:
##########
@@ -457,11 +454,7 @@ def change_points_from_json(change_points_json):
analyzed_json["attributes"],
)
- new_options = AnalysisOptions()
- new_options.window_len = analyzed_json["options"]["window_len"]
- new_options.max_pvalue = analyzed_json["options"]["max_pvalue"]
- new_options.min_magnitude = analyzed_json["options"]["min_magnitude"]
- new_options.orig_edivisive = analyzed_json["options"]["orig_edivisive"]
+ new_options = AnalysisOptions.model_validate(analyzed_json["options"])
new_change_points =
change_points_from_json(analyzed_json["change_points"])
Review Comment:
`AnalyzedSeries.to_json()` now uses Pydantic `model_dump(mode="json")`,
which serializes `change_points_timestamp` to an ISO8601 string. When that
payload is later passed into `AnalyzedSeries.from_json()` (especially after a
`json.dumps/json.loads` boundary), the current code assigns the raw string back
to `analyzed_series.change_points_timestamp` (annotated/used as `datetime`),
leaving the in-memory object in an inconsistent state.
--
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]
