-
Notifications
You must be signed in to change notification settings - Fork 451
New C++ API for timestamp/duration indices #9200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
Latest documentation preview deployed successfully.
Note: This comment is updated whenever you push a commit. |
@rerun-bot full-check |
Started a full build: https://github.com/rerun-io/rerun/actions/runs/13693743088 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great! Feeling somewhat strong about not having Collection::first
(see comment), but otherwise all good
The names for the functions in TimeColumn are somewhat inconsistent with those of RecordingStream.
Seems like a good opportunity to make things more similar to Rust? I figure you'll eventually want to move towards IndexColumn
? 🤔
Not sure what the exact question to the Reviewer is here 😄. Can we make things less inconsistent?
About the naming: I added a TODO in the parent issue |
@rerun-bot full-check |
Started a full build: https://github.com/rerun-io/rerun/actions/runs/13699399079 |
Related
What
Updated the C++ API to distinguish between absolute timestamps and and relative durations.
We've deprecated the following functions in
RecordingStream
, with the following replacements:set_time_sequence
->set_index_sequence
set_time
->set_index_duration
orset_index_timestamp
set_time_seconds
->set_index_duration_secs
orset_index_timestamp_seconds_since_epoch
set_time_nanos
->set_index_duration_nanos
orset_index_timestamp_nanos_since_epoch
TimeColumn
also has deprecated functions.Questions to reviewer
The names for the functions in
TimeColumn
are somewhat inconsistent with those ofRecordingStream
.Other notes
I also changed/broke the C API.
TODO