Skip to content
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

Lint and fix python SDK (Py)RecordingStream upcasting issues #8184

Merged
merged 9 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/snippets/all/concepts/explicit_recording.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
"""Just makes sure that explicit recordings actually work."""

import os

import rerun as rr

rec = rr.new_recording("rerun_example_explicit_recording")

rr.log("points", rr.Points3D([[0, 0, 0], [1, 1, 1]]), recording=rec)

dir = os.path.dirname(os.path.abspath(__file__))
rr.log_file_from_path(os.path.join(dir, "../../../../tests/assets/cube.glb"), recording=rec)
4 changes: 4 additions & 0 deletions docs/snippets/snippets.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
# You should only ever use this if the test isn't implemented and cannot yet be implemented
# for one or more specific SDKs.
[opt_out.run]
"concepts/explicit_recording" = [ # python-specific check
"cpp",
"rust",
]
"concepts/static/log_static" = [ # pseudo-code
"py",
"cpp",
Expand Down
2 changes: 1 addition & 1 deletion rerun_notebook/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 4 additions & 7 deletions rerun_py/rerun_sdk/rerun/_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ def log(
components=components,
num_instances=num_instances,
static=static,
recording=recording,
recording=recording, # NOLINT
)


Expand Down Expand Up @@ -253,9 +253,6 @@ def log_components(
)
static = True

# Convert to a native recording
recording = RecordingStream.to_native(recording)

instanced: dict[str, pa.Array] = {}

components = list(components)
Expand Down Expand Up @@ -294,7 +291,7 @@ def log_components(
entity_path,
components=instanced,
static_=static,
recording=recording,
recording=recording.to_native() if recording is not None else None,
)


Expand Down Expand Up @@ -358,7 +355,7 @@ def log_file_from_path(
Path(file_path),
entity_path_prefix=entity_path_prefix,
static_=static,
recording=recording,
recording=recording.to_native() if recording is not None else None,
)


Expand Down Expand Up @@ -419,7 +416,7 @@ def log_file_from_contents(
file_contents,
entity_path_prefix=entity_path_prefix,
static_=static,
recording=recording,
recording=recording.to_native() if recording is not None else None,
)


Expand Down
2 changes: 1 addition & 1 deletion rerun_py/rerun_sdk/rerun/_send_columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,5 +252,5 @@ def send_columns(
entity_path,
timelines={t.timeline_name(): t.as_arrow_array() for t in times},
components=components_args,
recording=recording,
recording=recording.to_native() if recording is not None else None,
)
2 changes: 1 addition & 1 deletion rerun_py/rerun_sdk/rerun/error_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def _send_warning_or_raise(
# TODO(jleibs): Context/stack should be its own component.
context_descriptor = _build_warning_context_string(skip_first=depth_to_user_code + 1)

log("rerun", TextLog(text=f"{message}\n{context_descriptor}", level="WARN"), recording=recording)
log("rerun", TextLog(text=f"{message}\n{context_descriptor}", level="WARN"), recording=recording) # NOLINT
_rerun_exception_ctx.sending_warning = False
else:
warnings.warn(
Expand Down
9 changes: 7 additions & 2 deletions rerun_py/rerun_sdk/rerun/legacy_notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def as_html(
if blueprint is not None:
output_stream.send_blueprint(blueprint, make_active=True) # type: ignore[attr-defined]

data_memory = memory_recording(recording=recording)
data_memory = memory_recording(recording=recording) # NOLINT
output_memory = output_stream.memory_recording() # type: ignore[attr-defined]

base64_data = base64.b64encode(output_memory.storage.concat_as_bytes(data_memory.storage)).decode("utf-8")
Expand Down Expand Up @@ -165,7 +165,12 @@ def legacy_notebook_show(

"""
html = as_html(
width=width, height=height, app_url=app_url, timeout_ms=timeout_ms, blueprint=blueprint, recording=recording
width=width,
height=height,
app_url=app_url,
timeout_ms=timeout_ms,
blueprint=blueprint,
recording=recording, # NOLINT
)
try:
from IPython.core.display import HTML
Expand Down
7 changes: 5 additions & 2 deletions rerun_py/rerun_sdk/rerun/memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ def memory_recording(recording: RecordingStream | None = None) -> MemoryRecordin

"""

recording = RecordingStream.to_native(recording)
return MemoryRecording(bindings.memory_recording(recording=recording))
return MemoryRecording(
bindings.memory_recording(
recording=recording.to_native() if recording is not None else None,
)
)


class MemoryRecording:
Expand Down
2 changes: 1 addition & 1 deletion rerun_py/rerun_sdk/rerun/notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,6 @@ def notebook_show(
width=width,
height=height,
blueprint=blueprint,
recording=recording,
recording=recording, # NOLINT
)
viewer.display()
24 changes: 12 additions & 12 deletions rerun_py/rerun_sdk/rerun/recording_stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def new_recording(
if spawn:
from rerun.sinks import spawn as _spawn

_spawn(recording=recording)
_spawn(recording=recording) # NOLINT

return recording

Expand Down Expand Up @@ -271,7 +271,7 @@ def __del__(self): # type: ignore[no-untyped-def]
#
# See: https://github.com/rerun-io/rerun/issues/6223 for context on why this is necessary.
if recording is not None and not recording.is_forked_child():
bindings.flush(blocking=False, recording=recording)
bindings.flush(blocking=False, recording=recording) # NOLINT


def binary_stream(recording: RecordingStream | None = None) -> BinaryStream:
Expand Down Expand Up @@ -307,9 +307,7 @@ def binary_stream(recording: RecordingStream | None = None) -> BinaryStream:
An object that can be used to flush or read the data.

"""

recording = RecordingStream.to_native(recording)
return BinaryStream(bindings.binary_stream(recording=recording))
return BinaryStream(bindings.binary_stream(recording=recording.to_native() if recording is not None else None))


class BinaryStream:
Expand Down Expand Up @@ -382,7 +380,7 @@ def is_enabled(
This can be controlled with the environment variable `RERUN` (e.g. `RERUN=on` or `RERUN=off`).

"""
return bindings.is_enabled(recording=RecordingStream.to_native(recording)) # type: ignore[no-any-return]
return bindings.is_enabled(recording=recording.to_native() if recording is not None else None) # type: ignore[no-any-return]


def get_application_id(
Expand All @@ -404,7 +402,7 @@ def get_application_id(
The application ID that this recording is associated with.

"""
app_id = bindings.get_application_id(recording=RecordingStream.to_native(recording))
app_id = bindings.get_application_id(recording=recording.to_native() if recording is not None else None)
return str(app_id) if app_id is not None else None


Expand Down Expand Up @@ -436,7 +434,7 @@ def get_recording_id(
The recording ID that this recording is logging to.

"""
rec_id = bindings.get_recording_id(recording=RecordingStream.to_native(recording))
rec_id = bindings.get_recording_id(recording=recording.to_native() if recording is not None else None)
return str(rec_id) if rec_id is not None else None


Expand Down Expand Up @@ -469,7 +467,7 @@ def get_data_recording(
The most appropriate recording to log data to, in the current context, if any.

"""
result = bindings.get_data_recording(recording=RecordingStream.to_native(recording))
result = bindings.get_data_recording(recording=recording.to_native() if recording is not None else None)
return RecordingStream(result) if result is not None else None


Expand Down Expand Up @@ -497,7 +495,7 @@ def set_global_data_recording(recording: RecordingStream) -> RecordingStream | N
The newly active global recording.

"""
result = bindings.set_global_data_recording(RecordingStream.to_native(recording))
result = bindings.set_global_data_recording(recording.to_native())
return RecordingStream(result) if result is not None else None


Expand All @@ -515,7 +513,7 @@ def get_thread_local_data_recording() -> RecordingStream | None:
return RecordingStream(result) if result is not None else None


def set_thread_local_data_recording(recording: RecordingStream) -> RecordingStream | None:
def set_thread_local_data_recording(recording: RecordingStream | None) -> RecordingStream | None:
"""
Replaces the currently active thread-local recording with the specified one.

Expand All @@ -525,7 +523,9 @@ def set_thread_local_data_recording(recording: RecordingStream) -> RecordingStre
The newly active thread-local recording.

"""
result = bindings.set_thread_local_data_recording(recording=RecordingStream.to_native(recording))
result = bindings.set_thread_local_data_recording(
recording=recording.to_native() if recording is not None else None,
)
return RecordingStream(result) if result is not None else None


Expand Down
61 changes: 38 additions & 23 deletions rerun_py/rerun_sdk/rerun/sinks.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ def connect(
)

return connect_tcp(
addr, flush_timeout_sec=flush_timeout_sec, default_blueprint=default_blueprint, recording=recording
addr,
flush_timeout_sec=flush_timeout_sec,
default_blueprint=default_blueprint,
recording=recording, # NOLINT
)


Expand Down Expand Up @@ -111,7 +114,7 @@ def connect_tcp(
logging.warning("Rerun is disabled - connect() call ignored")
return

application_id = get_application_id(recording=recording)
application_id = get_application_id(recording=recording) # NOLINT
if application_id is None:
raise ValueError(
"No application id found. You must call rerun.init before connecting to a viewer, or provide a recording."
Expand All @@ -124,10 +127,11 @@ def connect_tcp(
application_id=application_id, blueprint=default_blueprint
).storage

recording = RecordingStream.to_native(recording)

bindings.connect_tcp(
addr=addr, flush_timeout_sec=flush_timeout_sec, default_blueprint=blueprint_storage, recording=recording
addr=addr,
flush_timeout_sec=flush_timeout_sec,
default_blueprint=blueprint_storage,
recording=recording.to_native() if recording is not None else None,
)


Expand Down Expand Up @@ -163,7 +167,7 @@ def save(
logging.warning("Rerun is disabled - save() call ignored. You must call rerun.init before saving a recording.")
return

application_id = get_application_id(recording=recording)
application_id = get_application_id(recording=recording) # NOLINT
if application_id is None:
raise ValueError(
"No application id found. You must call rerun.init before connecting to a viewer, or provide a recording."
Expand All @@ -176,9 +180,11 @@ def save(
application_id=application_id, blueprint=default_blueprint
).storage

recording = RecordingStream.to_native(recording)

bindings.save(path=str(path), default_blueprint=blueprint_storage, recording=recording)
bindings.save(
path=str(path),
default_blueprint=blueprint_storage,
recording=recording.to_native() if recording is not None else None,
)


def stdout(default_blueprint: BlueprintLike | None = None, recording: RecordingStream | None = None) -> None:
Expand Down Expand Up @@ -210,7 +216,7 @@ def stdout(default_blueprint: BlueprintLike | None = None, recording: RecordingS
logging.warning("Rerun is disabled - save() call ignored. You must call rerun.init before saving a recording.")
return

application_id = get_application_id(recording=recording)
application_id = get_application_id(recording=recording) # NOLINT
if application_id is None:
raise ValueError(
"No application id found. You must call rerun.init before connecting to a viewer, or provide a recording."
Expand All @@ -223,8 +229,10 @@ def stdout(default_blueprint: BlueprintLike | None = None, recording: RecordingS
application_id=application_id, blueprint=default_blueprint
).storage

recording = RecordingStream.to_native(recording)
bindings.stdout(default_blueprint=blueprint_storage, recording=recording)
bindings.stdout(
default_blueprint=blueprint_storage,
recording=recording.to_native() if recording is not None else None,
)


def disconnect(recording: RecordingStream | None = None) -> None:
Expand All @@ -243,8 +251,9 @@ def disconnect(recording: RecordingStream | None = None) -> None:

"""

recording = RecordingStream.to_native(recording)
bindings.disconnect(recording=recording)
bindings.disconnect(
recording=recording.to_native() if recording is not None else None,
)


@deprecated(
Expand Down Expand Up @@ -309,7 +318,7 @@ def serve(
web_port=web_port,
ws_port=ws_port,
default_blueprint=default_blueprint,
recording=recording,
recording=recording, # NOLINT
server_memory_limit=server_memory_limit,
)

Expand Down Expand Up @@ -362,7 +371,7 @@ def serve_web(
logging.warning("Rerun is disabled - serve() call ignored")
return

application_id = get_application_id(recording=recording)
application_id = get_application_id(recording=recording) # NOLINT
if application_id is None:
raise ValueError(
"No application id found. You must call rerun.init before connecting to a viewer, or provide a recording."
Expand All @@ -375,15 +384,14 @@ def serve_web(
application_id=application_id, blueprint=default_blueprint
).storage

recording = RecordingStream.to_native(recording)
# TODO(#5531): keep static data around.
bindings.serve_web(
open_browser,
web_port,
ws_port,
server_memory_limit=server_memory_limit,
default_blueprint=blueprint_storage,
recording=recording,
recording=recording.to_native() if recording is not None else None,
)


Expand Down Expand Up @@ -417,16 +425,19 @@ def send_blueprint(
See also: [`rerun.init`][], [`rerun.set_global_data_recording`][].

"""
application_id = get_application_id(recording=recording)
application_id = get_application_id(recording=recording) # NOLINT

if application_id is None:
raise ValueError("No application id found. You must call rerun.init before sending a blueprint.")

recording = RecordingStream.to_native(recording)

blueprint_storage = create_in_memory_blueprint(application_id=application_id, blueprint=blueprint).storage

bindings.send_blueprint(blueprint_storage, make_active, make_default, recording=recording)
bindings.send_blueprint(
blueprint_storage,
make_active,
make_default,
recording=recording.to_native() if recording is not None else None,
)


def spawn(
Expand Down Expand Up @@ -477,4 +488,8 @@ def spawn(
_spawn_viewer(port=port, memory_limit=memory_limit, hide_welcome_screen=hide_welcome_screen)

if connect:
connect_tcp(f"127.0.0.1:{port}", recording=recording, default_blueprint=default_blueprint)
connect_tcp(
f"127.0.0.1:{port}",
recording=recording, # NOLINT
default_blueprint=default_blueprint,
)
Loading
Loading