Add experimental `send_recording` python api #9148

oxkitsune · 2025-02-26T20:36:38Z

What

Adds api to send a recording loaded from RRD to a new recording stream, cloning the data in the process

github-actions · 2025-02-26T20:38:07Z

Web viewer built successfully. If applicable, you should also test it:

I have tested the web viewer

Result	Commit	Link	Manifest
✅	`13562f5`	https://rerun.io/viewer/pr/9148	`+nightly` `+main`

^{Note: This comment is updated whenever you push a commit.}

Wumpf

Independent on whether we want to solve this differently in the future, the APIs are very valuable to have I believe.
There's some todos left though for documenting this out etc.
Also, I don't think we should close the linked issue until we implement this in all SDKs (right now the PR auto closes #2044)

rerun_py/src/python_bridge.rs

rerun_py/rerun_sdk/rerun/sinks.py

rerun_py/src/python_bridge.rs

rerun_py/src/dataframe.rs

teh-cmc · 2025-03-04T11:30:56Z

rerun_py/rerun_sdk/rerun/sinks.py

@@ -472,6 +473,31 @@ def send_blueprint(
    )


+def send_recording(recording: Recording, stream: RecordingStream | None = None) -> None:


Do we need this Python function at all? Can this logic be done in the Rust implementation instead? @jleibs has recently implemented a bunch of APIs for the Python SDK that don't have a single line of Python in them (AFAIK?), and I would very much like to keep that trend...

I looked into it, but it doesn’t seem super straightforward in this case. There are a few things that make it a bit tricky, e.g. RecordingStream wrapping PyRecordingStream

Yes. This can only be done correctly once everything else is done correctly. By "correctly", I mean according to this issue:

Project: first-class stateless API for the Python SDK #9187

abey79 · 2025-03-05T16:28:30Z

rerun_py/rerun_sdk/rerun/sinks.py

@@ -488,6 +490,31 @@ def send_blueprint(
    )


+def send_recording(embedded_recording: Recording, recording: RecordingStream | None = None) -> None:


This absolutely needs to also be a RecordingStream method.

Also, I don't know if this has been discussed, but our naming is all over the place, as this signature makes it explicit.

As they say, in for a penny, in for a pound. My actionable (if drastic) suggestion is either:

either rename all recording: RecordingStream arguments of the stateful API to stream: RecordingStream,

or preferably remove those arguments altogether! (if you want to act on an existing recording stream, just use one of its method)

I'm strongly advocating for the latter option.

cc @nikolausWest @jleibs

This is relevant: #9187

I'm late to the party on this one, but we should really think about the flow between the Recording and a RecordingStream object a bit more.

For example: the MemoryRecording would ideally go away and just be a Recording. Would be so nice to be able to log to the in-memory recording and then query it without needing to re-load it from an rrd.

Wumpf

.rrd isn't checked in as a LFS file, should be though

otherwise (and remaining comment) looks all good to me now. I'd be in favor of landing this prior to additional Python refactors, but I'll let you and @abey79 figure that out ;-)

Wumpf · 2025-03-06T11:27:19Z

docs/snippets/all/concepts/send_recording.rs

+    if let Some(info) = store.info().cloned() {
+        let new_recording = rerun::RecordingStreamBuilder::new(info.application_id.clone())
+            .recording_id(store_id.to_string())
+            .spawn()?;
+
+        new_recording.record_msg(LogMsg::SetStoreInfo(SetStoreInfo {
+            row_id: Tuid::new(),
+            info,
+        }));
+


[not actionable] oof this is pretty wild, going way more into Rerun's guts as a would like. Ah well, why not. At least this gives us a great reference point testing the python layer! 👍

docs/snippets/snippets.toml

docs/snippets/all/concepts/send_recording.py

Wumpf · 2025-03-06T11:31:22Z

rerun_py/docs/gen_common_index.py

@@ -89,6 +89,7 @@ class Section:
            "disconnect",
            "save",
            "send_blueprint",
+            "send_recording",


this isn't really an "initialize" function, right? All the others here are about setup, whereas send_recording is technically more like a logging function 🤔

oxkitsune · 2025-03-18T13:40:16Z

So I think this PR solves a problem we have, but I think the solution can be improved.

Currently all data is copied over, but it might make more sense to only take a small portion instead.
Would perhaps be nice to instead create a Dataframe from a RecordingStream kind of like Create Dataframe directly from RecordingStream #9302
Rust example makes use of too much of Rerun's internals for my liking

Wumpf

examples/assets/example.rrd is already checked in as an lfs file by now, this overrides it with an regular binary

rerun_py/src/python_bridge.rs

abey79

nice

oxkitsune added 🐍 Python API Python logging API include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages feat-dataframe-api Everything related to the dataframe API labels Feb 26, 2025

oxkitsune changed the title ~~Add send_recording python api~~ Add send_recording python api Feb 26, 2025

Wumpf self-requested a review March 3, 2025 11:23

Wumpf requested changes Mar 3, 2025

View reviewed changes

Wumpf assigned oxkitsune Mar 3, 2025

teh-cmc reviewed Mar 4, 2025

View reviewed changes

oxkitsune force-pushed the gijs/to-stream branch 3 times, most recently from b727ab8 to ffa7c43 Compare March 5, 2025 11:13

oxkitsune requested a review from Wumpf March 5, 2025 12:04

oxkitsune mentioned this pull request Mar 5, 2025

Update test and snippets using new_recording() to _actually_ use the stateless API #9196

Merged

abey79 requested changes Mar 5, 2025

View reviewed changes

Wumpf approved these changes Mar 6, 2025

View reviewed changes

Wumpf marked this pull request as draft March 19, 2025 08:51

oxkitsune force-pushed the gijs/to-stream branch 2 times, most recently from 17fbeac to eee8364 Compare March 31, 2025 07:18

Wumpf self-requested a review April 2, 2025 13:39

Wumpf requested changes Apr 2, 2025

View reviewed changes

rerun_py/src/python_bridge.rs Outdated Show resolved Hide resolved

oxkitsune added 7 commits April 3, 2025 09:34

send_recording api

071a955

fmt

aaad56c

fix ci

baf8a7a

feedback

880e3b4

add send_recording snippet

f185e1b

load rrd in snippet

be2376a

update example asset

f16204f

oxkitsune added 9 commits April 3, 2025 09:34

use dna rrd

8ea1570

document copied store info

cddc746

Add rust send_recording snippet

90bc78d

change

7729f20

Update send_recording api

800f913

Add clarifying comment for log_file

158bfe4

Do not copy over store id

bdfbb93

Revert pub(crate)

2294bb1

Restore file to git lfs

19776f9

oxkitsune force-pushed the gijs/to-stream branch from 0c1bf18 to 19776f9 Compare April 3, 2025 07:38

oxkitsune added 3 commits April 3, 2025 09:51

Add warnings + remove store info cloning from rust example

dd67217

remove unused imports

26c170f

ci

8ad1f8a

oxkitsune marked this pull request as ready for review April 3, 2025 08:22

Align doc signature

13562f5

Wumpf approved these changes Apr 3, 2025

View reviewed changes

oxkitsune requested a review from abey79 April 4, 2025 06:49

abey79 approved these changes Apr 4, 2025

View reviewed changes

oxkitsune changed the title ~~Add send_recording python api~~ Add experimental send_recording python api Apr 4, 2025

oxkitsune merged commit 68ecaee into main Apr 4, 2025
39 of 40 checks passed

oxkitsune deleted the gijs/to-stream branch April 4, 2025 07:27

emilk mentioned this pull request Apr 11, 2025

Release 0.23 #9588

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add experimental `send_recording` python api #9148

Add experimental `send_recording` python api #9148

oxkitsune commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025 •

edited

Loading

Wumpf left a comment •

edited

Loading

teh-cmc Mar 4, 2025

oxkitsune Mar 4, 2025

abey79 Mar 5, 2025

abey79 Mar 5, 2025

abey79 Mar 5, 2025

jleibs Mar 5, 2025

Wumpf left a comment

Wumpf Mar 6, 2025 •

edited

Loading

Wumpf Mar 6, 2025

oxkitsune commented Mar 18, 2025

Wumpf left a comment

abey79 left a comment

		@@ -472,6 +473,31 @@ def send_blueprint(
		)


		def send_recording(recording: Recording, stream: RecordingStream \| None = None) -> None:

		@@ -488,6 +490,31 @@ def send_blueprint(
		)


		def send_recording(embedded_recording: Recording, recording: RecordingStream \| None = None) -> None:

Add experimental send_recording python api #9148

Add experimental send_recording python api #9148

Conversation

oxkitsune commented Feb 26, 2025 • edited Loading

Related

What

github-actions bot commented Feb 26, 2025 • edited Loading

Wumpf left a comment • edited Loading

Choose a reason for hiding this comment

teh-cmc Mar 4, 2025

Choose a reason for hiding this comment

oxkitsune Mar 4, 2025

Choose a reason for hiding this comment

abey79 Mar 5, 2025

Choose a reason for hiding this comment

abey79 Mar 5, 2025

Choose a reason for hiding this comment

abey79 Mar 5, 2025

Choose a reason for hiding this comment

jleibs Mar 5, 2025

Choose a reason for hiding this comment

Wumpf left a comment

Choose a reason for hiding this comment

Wumpf Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Wumpf Mar 6, 2025

Choose a reason for hiding this comment

oxkitsune commented Mar 18, 2025

Wumpf left a comment

Choose a reason for hiding this comment

abey79 left a comment

Choose a reason for hiding this comment

Add experimental `send_recording` python api #9148

Add experimental `send_recording` python api #9148

oxkitsune commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025 •

edited

Loading

Wumpf left a comment •

edited

Loading

Wumpf Mar 6, 2025 •

edited

Loading