Add `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
✅	`e2c8996`	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

Wumpf · 2025-03-03T13:07:43Z

rerun_py/rerun_sdk/rerun/sinks.py

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


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


Needs to be mentioned in gen_common_index.py

Also, we need a snippet using this.
Ideally we'd add it to all languages if only to have the snippet comparision test do something - I figure at least for Rust we could whip up something?

I've added snippets for both rust/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! 👍

Wumpf · 2025-03-06T11:28:35Z

docs/snippets/snippets.toml

@@ -80,6 +80,9 @@ features = [
  "cpp",
  "rust",
 ]
+"concepts/send_recording" = [ # No output


Suggested change

"concepts/send_recording" = [ # No output

"concepts/send_recording" = [ # Not implemented for C++

ideally with issue link, but can let it slide

Wumpf · 2025-03-06T11:30:56Z

docs/snippets/all/concepts/send_recording.py

+
+path_to_rrd = sys.argv[1]
+
+recording = rr.dataframe.load_recording(path_to_rrd)


sorry for not mentioning earlier, but I think it's super important that we point out that if you just want to send an rrd file in, one should use log_file. This however is just a demonstration of glueing rr.dataframe in.
Already can see people copying this for loading RRDs 😬

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 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 2 times, most recently from d4e6138 to b727ab8 Compare March 5, 2025 11:06

oxkitsune added 9 commits March 5, 2025 12:08

send_recording api

9dfd6a2

fmt

0eedad7

fix ci

a0d725f

feedback

7c8b21a

add send_recording snippet

8f1afb3

load rrd in snippet

7fa4dab

update example asset

399d3e7

use dna rrd

776d0cf

document copied store info

ffa7c43

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

Add rust send_recording snippet

e2c8996

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `send_recording` python api #9148

Add `send_recording` python api #9148

oxkitsune commented Feb 26, 2025

github-actions bot commented Feb 26, 2025 •

edited

Loading

Wumpf left a comment •

edited

Loading

Wumpf Mar 3, 2025

Wumpf Mar 3, 2025

oxkitsune Mar 5, 2025

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

Wumpf Mar 6, 2025

Wumpf Mar 6, 2025

		@@ -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:

	"concepts/send_recording" = [ # No output
	"concepts/send_recording" = [ # Not implemented for C++


		path_to_rrd = sys.argv[1]

		recording = rr.dataframe.load_recording(path_to_rrd)

Add send_recording python api #9148

Are you sure you want to change the base?

Add send_recording python api #9148

Conversation

oxkitsune commented Feb 26, 2025

Related

What

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

Wumpf left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add `send_recording` python api #9148

Add `send_recording` python api #9148

github-actions bot commented Feb 26, 2025 •

edited

Loading

Wumpf left a comment •

edited

Loading

Wumpf Mar 6, 2025 •

edited

Loading