From c23e81e10f9510bab62ad81186b101a886072931 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 22 Oct 2024 15:39:29 +0200 Subject: [PATCH] Allow passing seconds/nanoseconds to `VideoFrameReference` archetype (#7833) ### What * Fixes #7822 Also, passing a float directly will trigger a warning now. ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7833?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7833?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7833) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --- .../all/archetypes/video_manual_frames.py | 10 +-- .../rerun_sdk/rerun/archetypes/asset_video.py | 10 +-- .../rerun/archetypes/video_frame_reference.py | 50 ++------------ .../archetypes/video_frame_reference_ext.py | 69 +++++++++++++++++++ .../rerun/components/video_timestamp_ext.py | 2 + .../tests/unit/test_video_frame_reference.py | 33 +++++++++ 6 files changed, 114 insertions(+), 60 deletions(-) create mode 100644 rerun_py/rerun_sdk/rerun/archetypes/video_frame_reference_ext.py create mode 100644 rerun_py/tests/unit/test_video_frame_reference.py diff --git a/docs/snippets/all/archetypes/video_manual_frames.py b/docs/snippets/all/archetypes/video_manual_frames.py index 9aadb1fa4fad..fb232223e7f2 100644 --- a/docs/snippets/all/archetypes/video_manual_frames.py +++ b/docs/snippets/all/archetypes/video_manual_frames.py @@ -19,17 +19,11 @@ # Create two entities, showing the same video frozen at different times. rr.log( "frame_1s", - rr.VideoFrameReference( - timestamp=rr.components.VideoTimestamp(seconds=1.0), - video_reference="video_asset", - ), + rr.VideoFrameReference(seconds=1.0, video_reference="video_asset"), ) rr.log( "frame_2s", - rr.VideoFrameReference( - timestamp=rr.components.VideoTimestamp(seconds=2.0), - video_reference="video_asset", - ), + rr.VideoFrameReference(seconds=2.0, video_reference="video_asset"), ) # Send blueprint that shows two 2D views next to each other. diff --git a/rerun_py/rerun_sdk/rerun/archetypes/asset_video.py b/rerun_py/rerun_sdk/rerun/archetypes/asset_video.py index f3ab25045295..f78337f6e421 100644 --- a/rerun_py/rerun_sdk/rerun/archetypes/asset_video.py +++ b/rerun_py/rerun_sdk/rerun/archetypes/asset_video.py @@ -90,17 +90,11 @@ class AssetVideo(AssetVideoExt, Archetype): # Create two entities, showing the same video frozen at different times. rr.log( "frame_1s", - rr.VideoFrameReference( - timestamp=rr.components.VideoTimestamp(seconds=1.0), - video_reference="video_asset", - ), + rr.VideoFrameReference(seconds=1.0, video_reference="video_asset"), ) rr.log( "frame_2s", - rr.VideoFrameReference( - timestamp=rr.components.VideoTimestamp(seconds=2.0), - video_reference="video_asset", - ), + rr.VideoFrameReference(seconds=2.0, video_reference="video_asset"), ) # Send blueprint that shows two 2D views next to each other. diff --git a/rerun_py/rerun_sdk/rerun/archetypes/video_frame_reference.py b/rerun_py/rerun_sdk/rerun/archetypes/video_frame_reference.py index 47b5c46975d0..6700d994b4dd 100644 --- a/rerun_py/rerun_sdk/rerun/archetypes/video_frame_reference.py +++ b/rerun_py/rerun_sdk/rerun/archetypes/video_frame_reference.py @@ -5,21 +5,19 @@ from __future__ import annotations -from typing import Any - from attrs import define, field -from .. import components, datatypes +from .. import components from .._baseclasses import ( Archetype, ) -from ..error_utils import catch_and_log_exceptions +from .video_frame_reference_ext import VideoFrameReferenceExt __all__ = ["VideoFrameReference"] @define(str=False, repr=False, init=False) -class VideoFrameReference(Archetype): +class VideoFrameReference(VideoFrameReferenceExt, Archetype): """ **Archetype**: References a single video frame. @@ -90,17 +88,11 @@ class VideoFrameReference(Archetype): # Create two entities, showing the same video frozen at different times. rr.log( "frame_1s", - rr.VideoFrameReference( - timestamp=rr.components.VideoTimestamp(seconds=1.0), - video_reference="video_asset", - ), + rr.VideoFrameReference(seconds=1.0, video_reference="video_asset"), ) rr.log( "frame_2s", - rr.VideoFrameReference( - timestamp=rr.components.VideoTimestamp(seconds=2.0), - video_reference="video_asset", - ), + rr.VideoFrameReference(seconds=2.0, video_reference="video_asset"), ) # Send blueprint that shows two 2D views next to each other. @@ -118,37 +110,7 @@ class VideoFrameReference(Archetype): """ - def __init__( - self: Any, timestamp: datatypes.VideoTimestampLike, *, video_reference: datatypes.EntityPathLike | None = None - ): - """ - Create a new instance of the VideoFrameReference archetype. - - Parameters - ---------- - timestamp: - References the closest video frame to this timestamp. - - Note that this uses the closest video frame instead of the latest at this timestamp - in order to be more forgiving of rounding errors for inprecise timestamp types. - video_reference: - Optional reference to an entity with a [`archetypes.AssetVideo`][rerun.archetypes.AssetVideo]. - - If none is specified, the video is assumed to be at the same entity. - Note that blueprint overrides on the referenced video will be ignored regardless, - as this is always interpreted as a reference to the data store. - - For a series of video frame references, it is recommended to specify this path only once - at the beginning of the series and then rely on latest-at query semantics to - keep the video reference active. - - """ - - # You can define your own __init__ function as a member of VideoFrameReferenceExt in video_frame_reference_ext.py - with catch_and_log_exceptions(context=self.__class__.__name__): - self.__attrs_init__(timestamp=timestamp, video_reference=video_reference) - return - self.__attrs_clear__() + # __init__ can be found in video_frame_reference_ext.py def __attrs_clear__(self) -> None: """Convenience method for calling `__attrs_init__` with all `None`s.""" diff --git a/rerun_py/rerun_sdk/rerun/archetypes/video_frame_reference_ext.py b/rerun_py/rerun_sdk/rerun/archetypes/video_frame_reference_ext.py new file mode 100644 index 000000000000..95d0f56fd485 --- /dev/null +++ b/rerun_py/rerun_sdk/rerun/archetypes/video_frame_reference_ext.py @@ -0,0 +1,69 @@ +from __future__ import annotations + +from typing import Any + +from .. import components, datatypes +from ..error_utils import _send_warning_or_raise, catch_and_log_exceptions + + +class VideoFrameReferenceExt: + """Extension for [VideoFrameReference][rerun.archetypes.VideoFrameReference].""" + + def __init__( + self: Any, + timestamp: datatypes.VideoTimestampLike | None = None, + *, + seconds: float | None = None, + nanoseconds: int | None = None, + video_reference: datatypes.EntityPathLike | None = None, + ) -> None: + """ + Create a new instance of the VideoFrameReference archetype. + + Parameters + ---------- + timestamp: + References the closest video frame to this timestamp. + + Note that this uses the closest video frame instead of the latest at this timestamp + in order to be more forgiving of rounding errors for inprecise timestamp types. + + Mutally exclusive with `seconds` and `nanoseconds`. + seconds: + Sets the timestamp to the given number of seconds. + + Mutally exclusive with `timestamp` and `nanoseconds`. + nanoseconds: + Sets the timestamp to the given number of nanoseconds. + + Mutally exclusive with `timestamp` and `seconds`. + video_reference: + Optional reference to an entity with a [`archetypes.AssetVideo`][rerun.archetypes.AssetVideo]. + + If none is specified, the video is assumed to be at the same entity. + Note that blueprint overrides on the referenced video will be ignored regardless, + as this is always interpreted as a reference to the data store. + + For a series of video frame references, it is recommended to specify this path only once + at the beginning of the series and then rely on latest-at query semantics to + keep the video reference active. + + """ + + with catch_and_log_exceptions(context=self.__class__.__name__): + if timestamp is None: + if seconds is None and nanoseconds is None: + raise ValueError("Either timestamp or seconds/nanoseconds must be specified.") + timestamp = components.VideoTimestamp(seconds=seconds, nanoseconds=nanoseconds) + elif seconds is not None or nanoseconds is not None: + raise ValueError("Cannot specify both `timestamp` and `seconds`/`nanoseconds`.") + elif isinstance(timestamp, float): + _send_warning_or_raise("Timestamp can't be specified as a float, use `seconds` instead.") + + self.__attrs_init__( + timestamp=timestamp, + video_reference=video_reference, + ) + return + + self.__attrs_clear__() diff --git a/rerun_py/rerun_sdk/rerun/components/video_timestamp_ext.py b/rerun_py/rerun_sdk/rerun/components/video_timestamp_ext.py index f7c11958ef6f..f64ac5338a99 100644 --- a/rerun_py/rerun_sdk/rerun/components/video_timestamp_ext.py +++ b/rerun_py/rerun_sdk/rerun/components/video_timestamp_ext.py @@ -38,6 +38,8 @@ def __init__( if nanoseconds is not None: raise ValueError("Cannot specify both `seconds` and `nanoseconds`.") nanoseconds = int(seconds * 1e9 + 0.5) + elif nanoseconds is None: + raise ValueError("Either `seconds` or `nanoseconds` must be specified.") self.__attrs_init__(timestamp_ns=nanoseconds) return diff --git a/rerun_py/tests/unit/test_video_frame_reference.py b/rerun_py/tests/unit/test_video_frame_reference.py new file mode 100644 index 000000000000..d0b284231aa1 --- /dev/null +++ b/rerun_py/tests/unit/test_video_frame_reference.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +import pytest +import rerun as rr + + +def test_video_frame_reference() -> None: + rr.set_strict_mode(True) + + # Too many args: + with pytest.raises(ValueError): + rr.VideoFrameReference(timestamp=rr.components.VideoTimestamp(seconds=12.3), seconds=12.3, nanoseconds=123) + with pytest.raises(ValueError): + rr.VideoFrameReference(seconds=12.3, nanoseconds=123) + with pytest.raises(ValueError): + rr.VideoFrameReference(timestamp=rr.components.VideoTimestamp(seconds=12.3), nanoseconds=123) + with pytest.raises(ValueError): + rr.VideoFrameReference(seconds=12.3, nanoseconds=123) + + # No args: + with pytest.raises(ValueError): + rr.VideoFrameReference() + + # Correct usages: + assert rr.VideoFrameReference(seconds=12.3).timestamp == rr.components.VideoTimestampBatch( + rr.components.VideoTimestamp(seconds=12.3) + ) + assert rr.VideoFrameReference(nanoseconds=123).timestamp == rr.components.VideoTimestampBatch( + rr.components.VideoTimestamp(nanoseconds=123) + ) + assert rr.VideoFrameReference( + timestamp=rr.components.VideoTimestamp(nanoseconds=123) + ).timestamp == rr.components.VideoTimestampBatch(rr.components.VideoTimestamp(nanoseconds=123))