diff --git a/python/changelog.d/857.fix.md b/python/changelog.d/857.fix.md new file mode 100644 index 000000000..357c35208 --- /dev/null +++ b/python/changelog.d/857.fix.md @@ -0,0 +1 @@ +Run, annotation, test step, and data import time bounds are nanosecond timestamps, matching channel data, so a time axis built from them lines up with `get_data`. diff --git a/python/lib/sift_client/_tests/resources/test_data_imports.py b/python/lib/sift_client/_tests/resources/test_data_imports.py index f3bb9c2ea..a14f712c1 100644 --- a/python/lib/sift_client/_tests/resources/test_data_imports.py +++ b/python/lib/sift_client/_tests/resources/test_data_imports.py @@ -9,6 +9,7 @@ import pandas as pd import pytest +from google.protobuf.timestamp_pb2 import Timestamp from sift.common.type.v1.channel_config_pb2 import ChannelConfig as ChannelConfigProto from sift.data_imports.v2.data_imports_pb2 import ( DATA_IMPORT_STATUS_FAILED, @@ -38,6 +39,7 @@ resolve_data_type_key, ) from sift_client._internal.util.time_column import infer_time_column +from sift_client._tests.sift_types.channel_time import assert_axis_matches_channel from sift_client.resources import DataImportAPI, DataImportAPIAsync from sift_client.sift_types.channel import ChannelDataType from sift_client.sift_types.data_import import ( @@ -1280,6 +1282,20 @@ def test_unset_optionals_are_none(self): assert data_import.data_start_time is None assert data_import.data_stop_time is None + def test_bounds_match_channel_timestamp_resolution(self): + """Import bounds use the same integer scale as channel data.""" + start = Timestamp(seconds=1735689600, nanos=123456789) + stop = Timestamp(seconds=1735689601, nanos=0) + proto = _data_import_proto() + proto.data_start_time.CopyFrom(start) + proto.data_stop_time.CopyFrom(stop) + + data_import = DataImport._from_proto(proto) + + assert_axis_matches_channel( + data_import.data_start_time, data_import.data_stop_time, start, stop + ) + class TestDataImportsGet: @pytest.mark.asyncio diff --git a/python/lib/sift_client/_tests/sift_types/channel_time.py b/python/lib/sift_client/_tests/sift_types/channel_time.py new file mode 100644 index 000000000..aab29d07b --- /dev/null +++ b/python/lib/sift_client/_tests/sift_types/channel_time.py @@ -0,0 +1,51 @@ +"""Shared check that a time axis matches the index ``get_data`` builds.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import numpy as np +import pandas as pd +from google.protobuf.any_pb2 import Any +from sift.common.type.v1.channel_data_type_pb2 import CHANNEL_DATA_TYPE_DOUBLE +from sift.data.v2.data_pb2 import DoubleValue, DoubleValues, Metadata + +from sift_client._internal.low_level_wrappers.data import DataLowLevelClient + +if TYPE_CHECKING: + from google.protobuf.timestamp_pb2 import Timestamp + + +def channel_index(*timestamps: Timestamp) -> pd.DatetimeIndex: + """Index ``get_data`` builds for double-channel samples at these times.""" + values = DoubleValues( + metadata=Metadata( + data_type=CHANNEL_DATA_TYPE_DOUBLE, + sampled_ms=0, + channel=Metadata.Channel(name="voltage"), + ), + values=[DoubleValue(timestamp=ts, value=1.0) for ts in timestamps], + ) + packed = Any() + packed.Pack(values) + # ``Pack`` writes ``type.googleapis.com/...``. The data parser reads the + # ``sift.data`` prefix the API sends. + packed.type_url = "sift.data.v2.DoubleValues" + frame = DataLowLevelClient.try_deserialize_channel_data(packed)["voltage"] + index = frame.index + assert isinstance(index, pd.DatetimeIndex) + return index + + +def _integer_view(index: pd.DatetimeIndex) -> list[int]: + """Integers in the index's own unit, the scale interpolation uses.""" + return [int(value) for value in np.asarray(index, dtype=np.int64)] + + +def assert_axis_matches_channel(start, stop, *timestamps: Timestamp) -> None: + """A ``date_range`` across these bounds uses the channel index scale.""" + samples = channel_index(*timestamps) + axis = pd.date_range(start, stop, periods=2) + assert str(samples.dtype).startswith("datetime64[ns") + assert axis.dtype == samples.dtype + assert _integer_view(axis) == _integer_view(samples) diff --git a/python/lib/sift_client/_tests/sift_types/test_annotation.py b/python/lib/sift_client/_tests/sift_types/test_annotation.py index 6ac3a751c..6778e4c84 100644 --- a/python/lib/sift_client/_tests/sift_types/test_annotation.py +++ b/python/lib/sift_client/_tests/sift_types/test_annotation.py @@ -4,9 +4,12 @@ from unittest.mock import MagicMock import pytest +from google.protobuf.timestamp_pb2 import Timestamp +from sift.annotations.v1.annotations_pb2 import Annotation as AnnotationProto from sift.channels.v3.channels_pb2 import Channel as ChannelProto from sift.common.type.v1.channel_data_type_pb2 import ChannelDataType as ChannelDataTypeProto +from sift_client._tests.sift_types.channel_time import assert_axis_matches_channel from sift_client.sift_types import Annotation from sift_client.sift_types.annotation import ( AnnotationCommentElement, @@ -249,3 +252,19 @@ def test_state_verbs_forward_to_the_resource(self, mock_annotation, mock_client) getattr(mock_client.annotations, verb).assert_called_once_with( annotation=mock_annotation ) + + +class TestAnnotationFromProto: + def test_bounds_match_channel_timestamp_resolution(self): + """Annotation bounds use the same integer scale as channel data.""" + start = Timestamp(seconds=1735689600, nanos=123456789) + stop = Timestamp(seconds=1735689601, nanos=0) + proto = AnnotationProto(annotation_id="a", name="n", description="") + proto.created_date.FromDatetime(datetime(2025, 1, 1, tzinfo=timezone.utc)) + proto.modified_date.CopyFrom(proto.created_date) + proto.start_time.CopyFrom(start) + proto.end_time.CopyFrom(stop) + + annotation = Annotation._from_proto(proto) + + assert_axis_matches_channel(annotation.start_time, annotation.end_time, start, stop) diff --git a/python/lib/sift_client/_tests/sift_types/test_results.py b/python/lib/sift_client/_tests/sift_types/test_results.py index c1530f1e7..1b50668bc 100644 --- a/python/lib/sift_client/_tests/sift_types/test_results.py +++ b/python/lib/sift_client/_tests/sift_types/test_results.py @@ -8,6 +8,7 @@ from unittest.mock import MagicMock, call import pytest +from google.protobuf.timestamp_pb2 import Timestamp from sift.test_reports.v1.test_reports_pb2 import ( TestMeasurement as TestMeasurementProto, ) @@ -15,6 +16,7 @@ TestStep as TestStepProto, ) +from sift_client._tests.sift_types.channel_time import assert_axis_matches_channel from sift_client.sift_types.channel import Channel, ChannelDataType from sift_client.sift_types.test_report import ( ErrorInfo, @@ -496,3 +498,22 @@ def test_step_from_proto_handles_absent_metadata(self): proto.end_time.FromDatetime(datetime.now(timezone.utc)) step = TestStep._from_proto(proto) assert step.metadata is None + + def test_bounds_match_channel_timestamp_resolution(self): + """Step bounds use the same integer scale as channel data.""" + start = Timestamp(seconds=1735689600, nanos=123456789) + stop = Timestamp(seconds=1735689601, nanos=0) + proto = TestStepProto( + test_step_id="step_abc", + test_report_id="report_789", + name="Step", + step_type=TestStepType.ACTION.value, + step_path="1", + status=TestStatus.IN_PROGRESS.value, + ) + proto.start_time.CopyFrom(start) + proto.end_time.CopyFrom(stop) + + step = TestStep._from_proto(proto) + + assert_axis_matches_channel(step.start_time, step.end_time, start, stop) diff --git a/python/lib/sift_client/_tests/sift_types/test_run.py b/python/lib/sift_client/_tests/sift_types/test_run.py index 58695efdf..b94b59f56 100644 --- a/python/lib/sift_client/_tests/sift_types/test_run.py +++ b/python/lib/sift_client/_tests/sift_types/test_run.py @@ -4,7 +4,10 @@ from unittest.mock import MagicMock import pytest +from google.protobuf.timestamp_pb2 import Timestamp +from sift.runs.v2.runs_pb2 import Run as RunProto +from sift_client._tests.sift_types.channel_time import assert_axis_matches_channel from sift_client.sift_types import Run from sift_client.sift_types.run import RunCreate, RunUpdate @@ -134,6 +137,45 @@ def mock_run(mock_client): class TestRun: """Unit tests for Run model - tests properties and methods.""" + def test_bounds_match_channel_timestamp_resolution(self): + """A time axis from run bounds matches the channel index for the same timestamps. + + ``stop_time`` is an exact second. That instant prints the same as a + microsecond ``datetime`` and still disagrees when both are read as + integers. + """ + start = Timestamp(seconds=1735689600, nanos=123456789) + stop = Timestamp(seconds=1735689601, nanos=0) + proto = RunProto( + run_id="r", + name="n", + description="", + created_by_user_id="u", + modified_by_user_id="u", + organization_id="o", + ) + proto.created_date.FromDatetime(datetime(2025, 1, 1, tzinfo=timezone.utc)) + proto.modified_date.CopyFrom(proto.created_date) + proto.start_time.CopyFrom(start) + proto.stop_time.CopyFrom(stop) + + run = Run._from_proto(proto) + assert_axis_matches_channel(run.start_time, run.stop_time, start, stop) + + unset = RunProto( + run_id="r", + name="n", + description="", + created_by_user_id="u", + modified_by_user_id="u", + organization_id="o", + ) + unset.created_date.CopyFrom(proto.created_date) + unset.modified_date.CopyFrom(proto.modified_date) + unbounded = Run._from_proto(unset) + assert unbounded.start_time is None + assert unbounded.stop_time is None + def test_assets_property_calls_client(self, mock_run, mock_client): """Test that assets property calls client.assets.list_ with correct parameters.""" mock_client.assets.list_.return_value = [] diff --git a/python/lib/sift_client/sift_types/annotation.py b/python/lib/sift_client/sift_types/annotation.py index 86f3d23c4..2cbb3e357 100644 --- a/python/lib/sift_client/sift_types/annotation.py +++ b/python/lib/sift_client/sift_types/annotation.py @@ -4,6 +4,7 @@ from enum import Enum from typing import TYPE_CHECKING, ClassVar, Literal, cast +import pandas as pd # noqa: TC002 from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator from sift.annotation_logs.v1.annotation_logs_pb2 import ( AnnotationCommentBodyElement as AnnotationCommentBodyElementProto, @@ -43,6 +44,7 @@ CreateAnnotationRequest as CreateAnnotationRequestProto, ) +from sift_client._internal.time import to_timestamp_nanos from sift_client.sift_types._base import ( BaseType, MappingHelper, @@ -105,13 +107,20 @@ def _linked_channel_to_proto(channel: Channel | CalculatedChannel) -> Annotation class Annotation(BaseType[AnnotationProto, "Annotation"]): - """Annotation model representing a time range on one or more assets.""" + """Annotation model representing a time range on one or more assets. + + ``start_time`` and ``end_time`` are nanosecond timestamps, the same + resolution as channel data from ``get_data``. + """ + + # pandas.Timestamp has no pydantic schema, so it is validated with isinstance. + model_config = ConfigDict(arbitrary_types_allowed=True) # Required fields name: str description: str - start_time: datetime - end_time: datetime + start_time: pd.Timestamp | datetime + end_time: pd.Timestamp | datetime annotation_type: AnnotationType organization_id: str created_date: datetime @@ -141,8 +150,8 @@ def _from_proto( id_=proto.annotation_id, name=proto.name, description=proto.description, - start_time=proto.start_time.ToDatetime(tzinfo=timezone.utc), - end_time=proto.end_time.ToDatetime(tzinfo=timezone.utc), + start_time=to_timestamp_nanos(proto.start_time), + end_time=to_timestamp_nanos(proto.end_time), annotation_type=AnnotationType(proto.annotation_type), organization_id=proto.organization_id, created_date=proto.created_date.ToDatetime(tzinfo=timezone.utc), diff --git a/python/lib/sift_client/sift_types/data_import.py b/python/lib/sift_client/sift_types/data_import.py index 86688177b..0cb4c8322 100644 --- a/python/lib/sift_client/sift_types/data_import.py +++ b/python/lib/sift_client/sift_types/data_import.py @@ -1436,8 +1436,14 @@ class DataImport(BaseType[DataImportProto, "DataImport"]): The format-specific import config is not modeled here; read it off ``data_import.proto`` (e.g. ``data_import.proto.csv_config``). + + ``data_start_time`` and ``data_stop_time`` are nanosecond timestamps, + the same resolution as channel data from ``get_data``. """ + # pandas.Timestamp has no pydantic schema, so it is validated with isinstance. + model_config = ConfigDict(arbitrary_types_allowed=True) + source_url: str | None status: DataImportStatus error_message: str | None @@ -1450,8 +1456,8 @@ class DataImport(BaseType[DataImportProto, "DataImport"]): report_id: str | None asset_id: str | None # Time range of the imported data, not of the import itself. - data_start_time: datetime | None - data_stop_time: datetime | None + data_start_time: pd.Timestamp | datetime | None + data_stop_time: pd.Timestamp | datetime | None @classmethod def _from_proto( @@ -1471,12 +1477,12 @@ def _from_proto( report_id=proto.report_id or None, asset_id=proto.asset_id or None, data_start_time=( - proto.data_start_time.ToDatetime(tzinfo=timezone.utc) + to_timestamp_nanos(proto.data_start_time) if proto.HasField("data_start_time") else None ), data_stop_time=( - proto.data_stop_time.ToDatetime(tzinfo=timezone.utc) + to_timestamp_nanos(proto.data_stop_time) if proto.HasField("data_stop_time") else None ), diff --git a/python/lib/sift_client/sift_types/run.py b/python/lib/sift_client/sift_types/run.py index b61968d7b..796db2ec4 100644 --- a/python/lib/sift_client/sift_types/run.py +++ b/python/lib/sift_client/sift_types/run.py @@ -3,10 +3,12 @@ from datetime import datetime, timedelta, timezone from typing import TYPE_CHECKING, ClassVar -from pydantic import model_validator +import pandas as pd # noqa: TC002 +from pydantic import ConfigDict, model_validator from sift.runs.v2.runs_pb2 import CreateRunRequest as CreateRunRequestProto from sift.runs.v2.runs_pb2 import Run as RunProto +from sift_client._internal.time import to_timestamp_nanos from sift_client.sift_types._base import ( BaseType, MappingHelper, @@ -31,7 +33,16 @@ class Run(BaseType[RunProto, "Run"], FileAttachmentsMixin): - """Run model representing a data collection run.""" + """Run model representing a data collection run. + + ``start_time`` and ``stop_time`` are nanosecond ``pandas.Timestamp`` + values, the same resolution as channel data from ``get_data``. A time + axis built from these lines up with that data when both are read as + integers. + """ + + # pandas.Timestamp has no pydantic schema, so it is validated with isinstance. + model_config = ConfigDict(arbitrary_types_allowed=True) # Required fields name: str @@ -50,8 +61,8 @@ class Run(BaseType[RunProto, "Run"], FileAttachmentsMixin): is_archived: bool # Optional fields - start_time: datetime | None - stop_time: datetime | None + start_time: pd.Timestamp | datetime | None + stop_time: pd.Timestamp | datetime | None duration: timedelta | None default_report_id: str | None client_key: str | None @@ -68,14 +79,10 @@ def _from_proto(cls, proto: RunProto, sift_client: SiftClient | None = None) -> modified_by_user_id=proto.modified_by_user_id, organization_id=proto.organization_id, start_time=( - proto.start_time.ToDatetime(tzinfo=timezone.utc) - if proto.HasField("start_time") - else None + to_timestamp_nanos(proto.start_time) if proto.HasField("start_time") else None ), stop_time=( - proto.stop_time.ToDatetime(tzinfo=timezone.utc) - if proto.HasField("stop_time") - else None + to_timestamp_nanos(proto.stop_time) if proto.HasField("stop_time") else None ), duration=(proto.duration.ToTimedelta() if proto.HasField("duration") else None), name=proto.name, diff --git a/python/lib/sift_client/sift_types/test_report.py b/python/lib/sift_client/sift_types/test_report.py index b8b1f2236..9c21d3779 100644 --- a/python/lib/sift_client/sift_types/test_report.py +++ b/python/lib/sift_client/sift_types/test_report.py @@ -5,7 +5,8 @@ from enum import Enum from typing import TYPE_CHECKING, ClassVar -from pydantic import field_validator +import pandas as pd # noqa: TC002 +from pydantic import ConfigDict, field_validator from sift.test_reports.v1.test_reports_pb2 import ( CreateTestReportRequest as CreateTestReportRequestProto, ) @@ -28,6 +29,7 @@ TestStep as TestStepProto, ) +from sift_client._internal.time import to_timestamp_nanos from sift_client.sift_types._base import ( BaseType, MappingHelper, @@ -155,7 +157,14 @@ def to_proto(self) -> TestStepProto: class TestStep(BaseType[TestStepProto, "TestStep"], FileAttachmentsMixin, SimulatedMixin): - """TestStep model representing a step in a test.""" + """TestStep model representing a step in a test. + + ``start_time`` and ``end_time`` are nanosecond timestamps, the same + resolution as channel data from ``get_data``. + """ + + # pandas.Timestamp has no pydantic schema, so it is validated with isinstance. + model_config = ConfigDict(arbitrary_types_allowed=True) test_report_id: str parent_step_id: str | None = None @@ -164,8 +173,8 @@ class TestStep(BaseType[TestStepProto, "TestStep"], FileAttachmentsMixin, Simula step_type: TestStepType step_path: str status: TestStatus - start_time: datetime - end_time: datetime + start_time: pd.Timestamp | datetime + end_time: pd.Timestamp | datetime error_info: ErrorInfo | None = None # NOTE: update() replaces this map wholesale. See TODO(metadata-mixin) in # sift_types/_mixins/metadata.py before adding keys at runtime. @@ -187,8 +196,8 @@ def _from_proto(cls, proto: TestStepProto, sift_client: SiftClient | None = None step_type=TestStepType(proto.step_type), step_path=proto.step_path, status=TestStatus(proto.status), - start_time=proto.start_time.ToDatetime(tzinfo=timezone.utc), - end_time=proto.end_time.ToDatetime(tzinfo=timezone.utc), + start_time=to_timestamp_nanos(proto.start_time), + end_time=to_timestamp_nanos(proto.end_time), error_info=ErrorInfo._from_proto(proto.error_info, sift_client) if proto.HasField("error_info") else None,