From c09d741bc8a405202d94b72bf7dca8eff8fee07c Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 19 Nov 2024 17:20:37 +0100 Subject: [PATCH 1/9] look for recording=recording, it's a bad sign --- scripts/lint.py | 108 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 86 insertions(+), 22 deletions(-) diff --git a/scripts/lint.py b/scripts/lint.py index 6ada62a3e54e..75527a5d0ef4 100755 --- a/scripts/lint.py +++ b/scripts/lint.py @@ -115,7 +115,10 @@ def lint_url(url: str) -> str | None: def lint_line( - line: str, prev_line: str | None, file_extension: str = "rs", is_in_docstring: bool = False + line: str, + prev_line: str | None, + file_extension: str = "rs", + is_in_docstring: bool = False, ) -> str | None: if line == "": return None @@ -139,7 +142,9 @@ def lint_line( return "It's 'GitHub', not 'github'" if re.search(r"[.a-zA-Z] [a-zA-Z]", line): - if r"\n " not in line: # Allow `\n `, which happens e.g. when markdown is embeedded in a string + if ( + r"\n " not in line + ): # Allow `\n `, which happens e.g. when markdown is embeedded in a string return "Found double space" if double_the.search(line.lower()): @@ -170,6 +175,9 @@ def lint_line( if re.search(r"\b3d\b", line): return "we prefer '3D' over '3d'" + if "recording=rec" in line: + return "you must cast the RecordingStream first: `recording=RecordingStream.to_native(recording)" + if "FIXME" in line: return "we prefer TODO over FIXME" @@ -236,25 +244,45 @@ def lint_line( line, ): app_id = m.group(2) - if not app_id.startswith("rerun_example_") and not app_id == "": + if ( + not app_id.startswith("rerun_example_") + and not app_id == "" + ): return f"All examples should have an app_id starting with 'rerun_example_'. Found '{app_id}'" # Methods that return Self should usually be marked #[inline] or #[inline(always)] since they indicate a builder. if re.search(r"\(mut self.*-> Self", line): - if prev_line_stripped != "#[inline]" and prev_line_stripped != "#[inline(always)]": + if ( + prev_line_stripped != "#[inline]" + and prev_line_stripped != "#[inline(always)]" + ): return "Builder methods impls should be marked #[inline]" # Deref impls should be marked #[inline] or #[inline(always)]. if "fn deref(&self)" in line or "fn deref_mut(&mut self)" in line: - if prev_line_stripped != "#[inline]" and prev_line_stripped != "#[inline(always)]": + if ( + prev_line_stripped != "#[inline]" + and prev_line_stripped != "#[inline(always)]" + ): return "Deref/DerefMut impls should be marked #[inline]" # Deref impls should be marked #[inline] or #[inline(always)]. if "fn as_ref(&self)" in line or "fn borrow(&self)" in line: - if prev_line_stripped != "#[inline]" and prev_line_stripped != "#[inline(always)]": + if ( + prev_line_stripped != "#[inline]" + and prev_line_stripped != "#[inline(always)]" + ): return "as_ref/borrow implementations should be marked #[inline]" - if any(s in line for s in (": &dyn std::any::Any", ": &mut dyn std::any::Any", ": &dyn Any", ": &mut dyn Any")): + if any( + s in line + for s in ( + ": &dyn std::any::Any", + ": &mut dyn std::any::Any", + ": &dyn Any", + ": &mut dyn Any", + ) + ): return """Functions should never take `&dyn std::any::Any` as argument since `&Box` itself implements `Any`, making it easy to accidentally pass the wrong object. Expect purpose defined traits instead.""" @@ -415,7 +443,9 @@ def test_lint_line() -> None: # ----------------------------------------------------------------------------- -re_declaration = re.compile(r"^\s*((pub(\(\w*\))? )?(async )?((impl|fn|struct|enum|union|trait|type)\b))") +re_declaration = re.compile( + r"^\s*((pub(\(\w*\))? )?(async )?((impl|fn|struct|enum|union|trait|type)\b))" +) re_attribute = re.compile(r"^\s*\#\[(error|derive|inline)") re_docstring = re.compile(r"^\s*///") @@ -435,7 +465,11 @@ def is_empty(line: str) -> bool: ) """Only for Rust files.""" - if re_declaration.match(line) or re_attribute.match(line) or re_docstring.match(line): + if ( + re_declaration.match(line) + or re_attribute.match(line) + or re_docstring.match(line) + ): line = line.strip() prev_line = prev_line.strip() @@ -476,7 +510,9 @@ def lint_vertical_spacing(lines_in: list[str]) -> tuple[list[str], list[str]]: line_nr = line_nr + 1 if prev_line is not None and is_missing_blank_line_between(prev_line, line): - errors.append(f"{line_nr}: for readability, add newline before `{line.strip()}`") + errors.append( + f"{line_nr}: for readability, add newline before `{line.strip()}`" + ) lines_out.append("\n") lines_out.append(line) @@ -579,7 +615,9 @@ def lint_workspace_deps(lines_in: list[str]) -> tuple[list[str], list[str]]: line_nr = line_nr + 1 if re_workspace_dep.search(line): - errors.append(f"{line_nr}: Rust examples should never depend on workspace information (`{line.strip()}`)") + errors.append( + f"{line_nr}: Rust examples should never depend on workspace information (`{line.strip()}`)" + ) lines_out.append("\n") lines_out.append(line) @@ -771,7 +809,9 @@ def test_split_words(): for input, expected in test_cases: actual = split_words(input) - assert actual == expected, f"Expected '{input}' to split into {expected}, got {actual}" + assert ( + actual == expected + ), f"Expected '{input}' to split into {expected}, got {actual}" def fix_header_casing(s: str) -> str: @@ -806,7 +846,11 @@ def is_acronym_or_pascal_case(s: str) -> bool: if last_punctuation: word = word.capitalize() last_punctuation = None - elif not inline_code_block and not word.startswith("`") and not word.startswith('"'): + elif ( + not inline_code_block + and not word.startswith("`") + and not word.startswith('"') + ): try: idx = force_capitalized_as_lower.index(word.lower()) except ValueError: @@ -817,7 +861,9 @@ def is_acronym_or_pascal_case(s: str) -> bool: word = word[:-1] elif idx is not None: word = force_capitalized[idx] - elif is_acronym_or_pascal_case(word) or any(c in ("_", "(", ".") for c in word): + elif is_acronym_or_pascal_case(word) or any( + c in ("_", "(", ".") for c in word + ): pass # acroym, PascalCase, code, … elif word.lower() in allow_capitalized_as_lower: pass @@ -913,7 +959,9 @@ def lint_markdown(filepath: str, source: SourceFile) -> tuple[list[str], list[st elif not in_frontmatter: new_line = fix_enforced_upper_case(line) if new_line != line: - errors.append(f"{line_nr}: Certain words should be capitalized. This should be '{new_line}'.") + errors.append( + f"{line_nr}: Certain words should be capitalized. This should be '{new_line}'." + ) line = new_line if in_example_readme and not in_metadata: @@ -931,7 +979,9 @@ def lint_markdown(filepath: str, source: SourceFile) -> tuple[list[str], list[st def lint_example_description(filepath: str, fm: Frontmatter) -> list[str]: # only applies to examples' readme - if not filepath.startswith("./examples/python") or not filepath.endswith("README.md"): + if not filepath.startswith("./examples/python") or not filepath.endswith( + "README.md" + ): return [] return [] @@ -1022,7 +1072,9 @@ def should_ignore_index(self, start_idx: int, end_idx: int | None = None) -> boo _index_to_line_nr(self.content, end_idx) if end_idx is not None else None, ) - def error(self, message: str, *, line_nr: int | None = None, index: int | None = None) -> str: + def error( + self, message: str, *, line_nr: int | None = None, index: int | None = None + ) -> str: """Construct an error message. If either `line_nr` or `index` is passed, it's used to indicate a line number.""" if line_nr is None and index is not None: line_nr = _index_to_line_nr(self.content, index) @@ -1093,7 +1145,11 @@ def lint_file(filepath: str, args: Any) -> int: if args.fix: source.rewrite(lines_out) - if not filepath.startswith("./examples/rust") and filepath != "./Cargo.toml" and filepath.endswith("Cargo.toml"): + if ( + not filepath.startswith("./examples/rust") + and filepath != "./Cargo.toml" + and filepath.endswith("Cargo.toml") + ): error = lint_workspace_lints(source.content) if error is not None: @@ -1135,11 +1191,15 @@ def lint_crate_docs(should_ignore: Callable[[Any], bool]) -> int: del listed_crates[crate_name] if not re.search(r"\b" + crate_name + r"\b", architecture_md): - print(f"{architecture_md_file}: missing documentation for crate {crate.name}") + print( + f"{architecture_md_file}: missing documentation for crate {crate.name}" + ) error_count += 1 for crate_name, line_nr in sorted(listed_crates.items(), key=lambda x: x[1]): - print(f"{architecture_md_file}:{line_nr}: crate name {crate_name} does not exist") + print( + f"{architecture_md_file}:{line_nr}: crate name {crate_name} does not exist" + ) error_count += 1 return error_count @@ -1238,7 +1298,9 @@ def main() -> None: "./rerun_js/web-viewer/re_viewer.js", ) - should_ignore = parse_gitignore(".gitignore") # TODO(#6730): parse all .gitignore files, not just top-level + should_ignore = parse_gitignore( + ".gitignore" + ) # TODO(#6730): parse all .gitignore files, not just top-level script_dirpath = os.path.dirname(os.path.realpath(__file__)) root_dirpath = os.path.abspath(f"{script_dirpath}/..") @@ -1261,7 +1323,9 @@ def main() -> None: extension = filename.split(".")[-1] if extension in extensions: filepath = os.path.join(root, filename) - filepath = os.path.join(".", os.path.relpath(filepath, root_dirpath)) + filepath = os.path.join( + ".", os.path.relpath(filepath, root_dirpath) + ) filepath = str(filepath).replace("\\", "/") if should_ignore(filepath) or filepath.startswith(exclude_paths): continue From ff46476151f303995f98e05993aa2be783a8862b Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 19 Nov 2024 17:37:16 +0100 Subject: [PATCH 2/9] pyfmt --- scripts/lint.py | 90 +++++++++++-------------------------------------- 1 file changed, 20 insertions(+), 70 deletions(-) diff --git a/scripts/lint.py b/scripts/lint.py index 75527a5d0ef4..13cc8a995885 100755 --- a/scripts/lint.py +++ b/scripts/lint.py @@ -142,9 +142,7 @@ def lint_line( return "It's 'GitHub', not 'github'" if re.search(r"[.a-zA-Z] [a-zA-Z]", line): - if ( - r"\n " not in line - ): # Allow `\n `, which happens e.g. when markdown is embeedded in a string + if r"\n " not in line: # Allow `\n `, which happens e.g. when markdown is embeedded in a string return "Found double space" if double_the.search(line.lower()): @@ -244,34 +242,22 @@ def lint_line( line, ): app_id = m.group(2) - if ( - not app_id.startswith("rerun_example_") - and not app_id == "" - ): + if not app_id.startswith("rerun_example_") and not app_id == "": return f"All examples should have an app_id starting with 'rerun_example_'. Found '{app_id}'" # Methods that return Self should usually be marked #[inline] or #[inline(always)] since they indicate a builder. if re.search(r"\(mut self.*-> Self", line): - if ( - prev_line_stripped != "#[inline]" - and prev_line_stripped != "#[inline(always)]" - ): + if prev_line_stripped != "#[inline]" and prev_line_stripped != "#[inline(always)]": return "Builder methods impls should be marked #[inline]" # Deref impls should be marked #[inline] or #[inline(always)]. if "fn deref(&self)" in line or "fn deref_mut(&mut self)" in line: - if ( - prev_line_stripped != "#[inline]" - and prev_line_stripped != "#[inline(always)]" - ): + if prev_line_stripped != "#[inline]" and prev_line_stripped != "#[inline(always)]": return "Deref/DerefMut impls should be marked #[inline]" # Deref impls should be marked #[inline] or #[inline(always)]. if "fn as_ref(&self)" in line or "fn borrow(&self)" in line: - if ( - prev_line_stripped != "#[inline]" - and prev_line_stripped != "#[inline(always)]" - ): + if prev_line_stripped != "#[inline]" and prev_line_stripped != "#[inline(always)]": return "as_ref/borrow implementations should be marked #[inline]" if any( @@ -443,9 +429,7 @@ def test_lint_line() -> None: # ----------------------------------------------------------------------------- -re_declaration = re.compile( - r"^\s*((pub(\(\w*\))? )?(async )?((impl|fn|struct|enum|union|trait|type)\b))" -) +re_declaration = re.compile(r"^\s*((pub(\(\w*\))? )?(async )?((impl|fn|struct|enum|union|trait|type)\b))") re_attribute = re.compile(r"^\s*\#\[(error|derive|inline)") re_docstring = re.compile(r"^\s*///") @@ -465,11 +449,7 @@ def is_empty(line: str) -> bool: ) """Only for Rust files.""" - if ( - re_declaration.match(line) - or re_attribute.match(line) - or re_docstring.match(line) - ): + if re_declaration.match(line) or re_attribute.match(line) or re_docstring.match(line): line = line.strip() prev_line = prev_line.strip() @@ -510,9 +490,7 @@ def lint_vertical_spacing(lines_in: list[str]) -> tuple[list[str], list[str]]: line_nr = line_nr + 1 if prev_line is not None and is_missing_blank_line_between(prev_line, line): - errors.append( - f"{line_nr}: for readability, add newline before `{line.strip()}`" - ) + errors.append(f"{line_nr}: for readability, add newline before `{line.strip()}`") lines_out.append("\n") lines_out.append(line) @@ -615,9 +593,7 @@ def lint_workspace_deps(lines_in: list[str]) -> tuple[list[str], list[str]]: line_nr = line_nr + 1 if re_workspace_dep.search(line): - errors.append( - f"{line_nr}: Rust examples should never depend on workspace information (`{line.strip()}`)" - ) + errors.append(f"{line_nr}: Rust examples should never depend on workspace information (`{line.strip()}`)") lines_out.append("\n") lines_out.append(line) @@ -809,9 +785,7 @@ def test_split_words(): for input, expected in test_cases: actual = split_words(input) - assert ( - actual == expected - ), f"Expected '{input}' to split into {expected}, got {actual}" + assert actual == expected, f"Expected '{input}' to split into {expected}, got {actual}" def fix_header_casing(s: str) -> str: @@ -846,11 +820,7 @@ def is_acronym_or_pascal_case(s: str) -> bool: if last_punctuation: word = word.capitalize() last_punctuation = None - elif ( - not inline_code_block - and not word.startswith("`") - and not word.startswith('"') - ): + elif not inline_code_block and not word.startswith("`") and not word.startswith('"'): try: idx = force_capitalized_as_lower.index(word.lower()) except ValueError: @@ -861,9 +831,7 @@ def is_acronym_or_pascal_case(s: str) -> bool: word = word[:-1] elif idx is not None: word = force_capitalized[idx] - elif is_acronym_or_pascal_case(word) or any( - c in ("_", "(", ".") for c in word - ): + elif is_acronym_or_pascal_case(word) or any(c in ("_", "(", ".") for c in word): pass # acroym, PascalCase, code, … elif word.lower() in allow_capitalized_as_lower: pass @@ -959,9 +927,7 @@ def lint_markdown(filepath: str, source: SourceFile) -> tuple[list[str], list[st elif not in_frontmatter: new_line = fix_enforced_upper_case(line) if new_line != line: - errors.append( - f"{line_nr}: Certain words should be capitalized. This should be '{new_line}'." - ) + errors.append(f"{line_nr}: Certain words should be capitalized. This should be '{new_line}'.") line = new_line if in_example_readme and not in_metadata: @@ -979,9 +945,7 @@ def lint_markdown(filepath: str, source: SourceFile) -> tuple[list[str], list[st def lint_example_description(filepath: str, fm: Frontmatter) -> list[str]: # only applies to examples' readme - if not filepath.startswith("./examples/python") or not filepath.endswith( - "README.md" - ): + if not filepath.startswith("./examples/python") or not filepath.endswith("README.md"): return [] return [] @@ -1072,9 +1036,7 @@ def should_ignore_index(self, start_idx: int, end_idx: int | None = None) -> boo _index_to_line_nr(self.content, end_idx) if end_idx is not None else None, ) - def error( - self, message: str, *, line_nr: int | None = None, index: int | None = None - ) -> str: + def error(self, message: str, *, line_nr: int | None = None, index: int | None = None) -> str: """Construct an error message. If either `line_nr` or `index` is passed, it's used to indicate a line number.""" if line_nr is None and index is not None: line_nr = _index_to_line_nr(self.content, index) @@ -1145,11 +1107,7 @@ def lint_file(filepath: str, args: Any) -> int: if args.fix: source.rewrite(lines_out) - if ( - not filepath.startswith("./examples/rust") - and filepath != "./Cargo.toml" - and filepath.endswith("Cargo.toml") - ): + if not filepath.startswith("./examples/rust") and filepath != "./Cargo.toml" and filepath.endswith("Cargo.toml"): error = lint_workspace_lints(source.content) if error is not None: @@ -1191,15 +1149,11 @@ def lint_crate_docs(should_ignore: Callable[[Any], bool]) -> int: del listed_crates[crate_name] if not re.search(r"\b" + crate_name + r"\b", architecture_md): - print( - f"{architecture_md_file}: missing documentation for crate {crate.name}" - ) + print(f"{architecture_md_file}: missing documentation for crate {crate.name}") error_count += 1 for crate_name, line_nr in sorted(listed_crates.items(), key=lambda x: x[1]): - print( - f"{architecture_md_file}:{line_nr}: crate name {crate_name} does not exist" - ) + print(f"{architecture_md_file}:{line_nr}: crate name {crate_name} does not exist") error_count += 1 return error_count @@ -1298,9 +1252,7 @@ def main() -> None: "./rerun_js/web-viewer/re_viewer.js", ) - should_ignore = parse_gitignore( - ".gitignore" - ) # TODO(#6730): parse all .gitignore files, not just top-level + should_ignore = parse_gitignore(".gitignore") # TODO(#6730): parse all .gitignore files, not just top-level script_dirpath = os.path.dirname(os.path.realpath(__file__)) root_dirpath = os.path.abspath(f"{script_dirpath}/..") @@ -1323,9 +1275,7 @@ def main() -> None: extension = filename.split(".")[-1] if extension in extensions: filepath = os.path.join(root, filename) - filepath = os.path.join( - ".", os.path.relpath(filepath, root_dirpath) - ) + filepath = os.path.join(".", os.path.relpath(filepath, root_dirpath)) filepath = str(filepath).replace("\\", "/") if should_ignore(filepath) or filepath.startswith(exclude_paths): continue From e78d52884c38c6a5f356870f605f868a9f168222 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 19 Nov 2024 17:37:41 +0100 Subject: [PATCH 3/9] fix all instances of broken upcasts --- rerun_py/rerun_sdk/rerun/_log.py | 8 ++-- rerun_py/rerun_sdk/rerun/_send_columns.py | 2 +- rerun_py/rerun_sdk/rerun/error_utils.py | 2 +- rerun_py/rerun_sdk/rerun/legacy_notebook.py | 9 +++- rerun_py/rerun_sdk/rerun/memory.py | 3 +- rerun_py/rerun_sdk/rerun/notebook.py | 2 +- rerun_py/rerun_sdk/rerun/recording_stream.py | 8 ++-- rerun_py/rerun_sdk/rerun/sinks.py | 49 +++++++++++--------- rerun_py/rerun_sdk/rerun/time.py | 18 ++----- tests/python/gil_stress/main.py | 20 ++++---- 10 files changed, 59 insertions(+), 62 deletions(-) diff --git a/rerun_py/rerun_sdk/rerun/_log.py b/rerun_py/rerun_sdk/rerun/_log.py index 592bc03a9c64..9e8be9343c7d 100644 --- a/rerun_py/rerun_sdk/rerun/_log.py +++ b/rerun_py/rerun_sdk/rerun/_log.py @@ -179,7 +179,7 @@ def log( components=components, num_instances=num_instances, static=static, - recording=recording, + recording=recording, # NOLINT ) @@ -294,7 +294,7 @@ def log_components( entity_path, components=instanced, static_=static, - recording=recording, + recording=RecordingStream.to_native(recording), ) @@ -358,7 +358,7 @@ def log_file_from_path( Path(file_path), entity_path_prefix=entity_path_prefix, static_=static, - recording=recording, + recording=RecordingStream.to_native(recording), ) @@ -419,7 +419,7 @@ def log_file_from_contents( file_contents, entity_path_prefix=entity_path_prefix, static_=static, - recording=recording, + recording=RecordingStream.to_native(recording), ) diff --git a/rerun_py/rerun_sdk/rerun/_send_columns.py b/rerun_py/rerun_sdk/rerun/_send_columns.py index 945c1e3d8349..a8a1b2e1a347 100644 --- a/rerun_py/rerun_sdk/rerun/_send_columns.py +++ b/rerun_py/rerun_sdk/rerun/_send_columns.py @@ -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=RecordingStream.to_native(recording), ) diff --git a/rerun_py/rerun_sdk/rerun/error_utils.py b/rerun_py/rerun_sdk/rerun/error_utils.py index 6b8059a5c9ce..bd4cdbcbcb16 100644 --- a/rerun_py/rerun_sdk/rerun/error_utils.py +++ b/rerun_py/rerun_sdk/rerun/error_utils.py @@ -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( diff --git a/rerun_py/rerun_sdk/rerun/legacy_notebook.py b/rerun_py/rerun_sdk/rerun/legacy_notebook.py index cb4fcef7c63d..730ffb992a14 100644 --- a/rerun_py/rerun_sdk/rerun/legacy_notebook.py +++ b/rerun_py/rerun_sdk/rerun/legacy_notebook.py @@ -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") @@ -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 diff --git a/rerun_py/rerun_sdk/rerun/memory.py b/rerun_py/rerun_sdk/rerun/memory.py index bfa55afafb18..1048caffcf42 100644 --- a/rerun_py/rerun_sdk/rerun/memory.py +++ b/rerun_py/rerun_sdk/rerun/memory.py @@ -28,8 +28,7 @@ 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=RecordingStream.to_native(recording))) class MemoryRecording: diff --git a/rerun_py/rerun_sdk/rerun/notebook.py b/rerun_py/rerun_sdk/rerun/notebook.py index 6d79b6c97113..0fc161ebe4d6 100644 --- a/rerun_py/rerun_sdk/rerun/notebook.py +++ b/rerun_py/rerun_sdk/rerun/notebook.py @@ -190,6 +190,6 @@ def notebook_show( width=width, height=height, blueprint=blueprint, - recording=recording, + recording=recording, # NOLINT ) viewer.display() diff --git a/rerun_py/rerun_sdk/rerun/recording_stream.py b/rerun_py/rerun_sdk/rerun/recording_stream.py index fa58de4c60e0..7aebeb8001d6 100644 --- a/rerun_py/rerun_sdk/rerun/recording_stream.py +++ b/rerun_py/rerun_sdk/rerun/recording_stream.py @@ -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 @@ -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: @@ -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=RecordingStream.to_native(recording))) class BinaryStream: diff --git a/rerun_py/rerun_sdk/rerun/sinks.py b/rerun_py/rerun_sdk/rerun/sinks.py index 0d1a3e984b22..2e713c9156bb 100644 --- a/rerun_py/rerun_sdk/rerun/sinks.py +++ b/rerun_py/rerun_sdk/rerun/sinks.py @@ -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 ) @@ -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." @@ -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=RecordingStream.to_native(recording), ) @@ -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." @@ -176,9 +180,7 @@ 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=RecordingStream.to_native(recording)) def stdout(default_blueprint: BlueprintLike | None = None, recording: RecordingStream | None = None) -> None: @@ -210,7 +212,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." @@ -223,8 +225,7 @@ 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=RecordingStream.to_native(recording)) def disconnect(recording: RecordingStream | None = None) -> None: @@ -243,8 +244,7 @@ def disconnect(recording: RecordingStream | None = None) -> None: """ - recording = RecordingStream.to_native(recording) - bindings.disconnect(recording=recording) + bindings.disconnect(recording=RecordingStream.to_native(recording)) @deprecated( @@ -309,7 +309,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, ) @@ -362,7 +362,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." @@ -375,7 +375,6 @@ 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, @@ -383,7 +382,7 @@ def serve_web( ws_port, server_memory_limit=server_memory_limit, default_blueprint=blueprint_storage, - recording=recording, + recording=RecordingStream.to_native(recording), ) @@ -417,16 +416,16 @@ 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=RecordingStream.to_native(recording) + ) def spawn( @@ -477,4 +476,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, + ) diff --git a/rerun_py/rerun_sdk/rerun/time.py b/rerun_py/rerun_sdk/rerun/time.py index bb8775a4eafb..12cad17b805c 100644 --- a/rerun_py/rerun_sdk/rerun/time.py +++ b/rerun_py/rerun_sdk/rerun/time.py @@ -32,8 +32,7 @@ def set_time_sequence(timeline: str, sequence: int, recording: RecordingStream | See also: [`rerun.init`][], [`rerun.set_global_data_recording`][]. """ - recording = RecordingStream.to_native(recording) - bindings.set_time_sequence(timeline, sequence, recording=recording) + bindings.set_time_sequence(timeline, sequence, recording=RecordingStream.to_native(recording)) def set_time_seconds(timeline: str, seconds: float, recording: RecordingStream | None = None) -> None: @@ -69,8 +68,7 @@ def set_time_seconds(timeline: str, seconds: float, recording: RecordingStream | """ - recording = RecordingStream.to_native(recording) - bindings.set_time_seconds(timeline, seconds, recording=recording) + bindings.set_time_seconds(timeline, seconds, recording=RecordingStream.to_native(recording)) def set_time_nanos(timeline: str, nanos: int, recording: RecordingStream | None = None) -> None: @@ -106,9 +104,7 @@ def set_time_nanos(timeline: str, nanos: int, recording: RecordingStream | None """ - recording = RecordingStream.to_native(recording) - - bindings.set_time_nanos(timeline, nanos, recording=recording) + bindings.set_time_nanos(timeline, nanos, recording=RecordingStream.to_native(recording)) def disable_timeline(timeline: str, recording: RecordingStream | None = None) -> None: @@ -126,9 +122,7 @@ def disable_timeline(timeline: str, recording: RecordingStream | None = None) -> """ - recording = RecordingStream.to_native(recording) - - bindings.disable_timeline(timeline, recording=recording) + bindings.disable_timeline(timeline, recording=RecordingStream.to_native(recording)) def reset_time(recording: RecordingStream | None = None) -> None: @@ -149,6 +143,4 @@ def reset_time(recording: RecordingStream | None = None) -> None: """ - recording = RecordingStream.to_native(recording) - - bindings.reset_time(recording=recording) + bindings.reset_time(recording=RecordingStream.to_native(recording)) diff --git a/tests/python/gil_stress/main.py b/tests/python/gil_stress/main.py index b3a2152b6d00..bd1d3c44c52a 100644 --- a/tests/python/gil_stress/main.py +++ b/tests/python/gil_stress/main.py @@ -15,38 +15,38 @@ rec = rr.new_recording(application_id="test") rec = rr.new_recording(application_id="test") -rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) +rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) rec = rr.new_recording(application_id="test", make_default=True) -rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) +rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) rec = rr.new_recording(application_id="test", make_thread_default=True) -rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) +rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) rec = rr.new_recording(application_id="test") # this works rr.set_global_data_recording(rec) -rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) +rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) rec = rr.new_recording(application_id="test") # this works rr.set_thread_local_data_recording(rec) -rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) +rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) rec = rr.new_recording(application_id="test", spawn=True) -rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) +rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) rec = rr.new_recording(application_id="test") rr.connect_tcp(recording=rec) -rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) +rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) rec = rr.new_recording(application_id="test") rr.memory_recording(recording=rec) -rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) +rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) for _ in range(3): rec = rr.new_recording(application_id="test", make_default=False, make_thread_default=False) mem = rec.memory_recording() - rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) + rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) for _ in range(3): rec = rr.new_recording(application_id="test", make_default=False, make_thread_default=False) - rr.log("test", rr.Points3D([1, 2, 3]), recording=rec.inner) + rr.log("test", rr.Points3D([1, 2, 3]), recording=rec) From 532c4fd885454ee89fcbd678bdade3acabc1a62d Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 19 Nov 2024 17:43:04 +0100 Subject: [PATCH 4/9] RecordingStream.to_native(recording) -> recording.to_native() --- rerun_notebook/package-lock.json | 2 +- rerun_py/rerun_sdk/rerun/_log.py | 8 +++--- rerun_py/rerun_sdk/rerun/_send_columns.py | 2 +- rerun_py/rerun_sdk/rerun/memory.py | 2 +- rerun_py/rerun_sdk/rerun/recording_stream.py | 14 +++++----- rerun_py/rerun_sdk/rerun/sinks.py | 14 +++++----- rerun_py/rerun_sdk/rerun/time.py | 10 +++---- scripts/lint.py | 28 ++++++++++++-------- 8 files changed, 42 insertions(+), 38 deletions(-) diff --git a/rerun_notebook/package-lock.json b/rerun_notebook/package-lock.json index fb15f46c36d8..98ffe5ec105e 100644 --- a/rerun_notebook/package-lock.json +++ b/rerun_notebook/package-lock.json @@ -17,7 +17,7 @@ }, "../rerun_js/web-viewer": { "name": "@rerun-io/web-viewer", - "version": "0.20.0-alpha.1+dev", + "version": "0.20.1-alpha.2", "license": "MIT", "devDependencies": { "dts-buddy": "^0.3.0", diff --git a/rerun_py/rerun_sdk/rerun/_log.py b/rerun_py/rerun_sdk/rerun/_log.py index 9e8be9343c7d..540e9567aa51 100644 --- a/rerun_py/rerun_sdk/rerun/_log.py +++ b/rerun_py/rerun_sdk/rerun/_log.py @@ -254,7 +254,7 @@ def log_components( static = True # Convert to a native recording - recording = RecordingStream.to_native(recording) + recording = recording.to_native() instanced: dict[str, pa.Array] = {} @@ -294,7 +294,7 @@ def log_components( entity_path, components=instanced, static_=static, - recording=RecordingStream.to_native(recording), + recording=recording.to_native(), ) @@ -358,7 +358,7 @@ def log_file_from_path( Path(file_path), entity_path_prefix=entity_path_prefix, static_=static, - recording=RecordingStream.to_native(recording), + recording=recording.to_native(), ) @@ -419,7 +419,7 @@ def log_file_from_contents( file_contents, entity_path_prefix=entity_path_prefix, static_=static, - recording=RecordingStream.to_native(recording), + recording=recording.to_native(), ) diff --git a/rerun_py/rerun_sdk/rerun/_send_columns.py b/rerun_py/rerun_sdk/rerun/_send_columns.py index a8a1b2e1a347..1cf8726060e6 100644 --- a/rerun_py/rerun_sdk/rerun/_send_columns.py +++ b/rerun_py/rerun_sdk/rerun/_send_columns.py @@ -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=RecordingStream.to_native(recording), + recording=recording.to_native(), ) diff --git a/rerun_py/rerun_sdk/rerun/memory.py b/rerun_py/rerun_sdk/rerun/memory.py index 1048caffcf42..16eec0e8ad2c 100644 --- a/rerun_py/rerun_sdk/rerun/memory.py +++ b/rerun_py/rerun_sdk/rerun/memory.py @@ -28,7 +28,7 @@ def memory_recording(recording: RecordingStream | None = None) -> MemoryRecordin """ - return MemoryRecording(bindings.memory_recording(recording=RecordingStream.to_native(recording))) + return MemoryRecording(bindings.memory_recording(recording=recording.to_native())) class MemoryRecording: diff --git a/rerun_py/rerun_sdk/rerun/recording_stream.py b/rerun_py/rerun_sdk/rerun/recording_stream.py index 7aebeb8001d6..4522e16d7642 100644 --- a/rerun_py/rerun_sdk/rerun/recording_stream.py +++ b/rerun_py/rerun_sdk/rerun/recording_stream.py @@ -307,7 +307,7 @@ def binary_stream(recording: RecordingStream | None = None) -> BinaryStream: An object that can be used to flush or read the data. """ - return BinaryStream(bindings.binary_stream(recording=RecordingStream.to_native(recording))) + return BinaryStream(bindings.binary_stream(recording=recording.to_native())) class BinaryStream: @@ -380,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()) # type: ignore[no-any-return] def get_application_id( @@ -402,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()) return str(app_id) if app_id is not None else None @@ -434,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()) return str(rec_id) if rec_id is not None else None @@ -467,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()) return RecordingStream(result) if result is not None else None @@ -495,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 @@ -523,7 +523,7 @@ 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()) return RecordingStream(result) if result is not None else None diff --git a/rerun_py/rerun_sdk/rerun/sinks.py b/rerun_py/rerun_sdk/rerun/sinks.py index 2e713c9156bb..df223da6d4be 100644 --- a/rerun_py/rerun_sdk/rerun/sinks.py +++ b/rerun_py/rerun_sdk/rerun/sinks.py @@ -131,7 +131,7 @@ def connect_tcp( addr=addr, flush_timeout_sec=flush_timeout_sec, default_blueprint=blueprint_storage, - recording=RecordingStream.to_native(recording), + recording=recording.to_native(), ) @@ -180,7 +180,7 @@ def save( application_id=application_id, blueprint=default_blueprint ).storage - bindings.save(path=str(path), default_blueprint=blueprint_storage, recording=RecordingStream.to_native(recording)) + bindings.save(path=str(path), default_blueprint=blueprint_storage, recording=recording.to_native()) def stdout(default_blueprint: BlueprintLike | None = None, recording: RecordingStream | None = None) -> None: @@ -225,7 +225,7 @@ def stdout(default_blueprint: BlueprintLike | None = None, recording: RecordingS application_id=application_id, blueprint=default_blueprint ).storage - bindings.stdout(default_blueprint=blueprint_storage, recording=RecordingStream.to_native(recording)) + bindings.stdout(default_blueprint=blueprint_storage, recording=recording.to_native()) def disconnect(recording: RecordingStream | None = None) -> None: @@ -244,7 +244,7 @@ def disconnect(recording: RecordingStream | None = None) -> None: """ - bindings.disconnect(recording=RecordingStream.to_native(recording)) + bindings.disconnect(recording=recording.to_native()) @deprecated( @@ -382,7 +382,7 @@ def serve_web( ws_port, server_memory_limit=server_memory_limit, default_blueprint=blueprint_storage, - recording=RecordingStream.to_native(recording), + recording=recording.to_native(), ) @@ -423,9 +423,7 @@ def send_blueprint( blueprint_storage = create_in_memory_blueprint(application_id=application_id, blueprint=blueprint).storage - bindings.send_blueprint( - blueprint_storage, make_active, make_default, recording=RecordingStream.to_native(recording) - ) + bindings.send_blueprint(blueprint_storage, make_active, make_default, recording=recording.to_native()) def spawn( diff --git a/rerun_py/rerun_sdk/rerun/time.py b/rerun_py/rerun_sdk/rerun/time.py index 12cad17b805c..d1cc52e8db50 100644 --- a/rerun_py/rerun_sdk/rerun/time.py +++ b/rerun_py/rerun_sdk/rerun/time.py @@ -32,7 +32,7 @@ def set_time_sequence(timeline: str, sequence: int, recording: RecordingStream | See also: [`rerun.init`][], [`rerun.set_global_data_recording`][]. """ - bindings.set_time_sequence(timeline, sequence, recording=RecordingStream.to_native(recording)) + bindings.set_time_sequence(timeline, sequence, recording=recording.to_native()) def set_time_seconds(timeline: str, seconds: float, recording: RecordingStream | None = None) -> None: @@ -68,7 +68,7 @@ def set_time_seconds(timeline: str, seconds: float, recording: RecordingStream | """ - bindings.set_time_seconds(timeline, seconds, recording=RecordingStream.to_native(recording)) + bindings.set_time_seconds(timeline, seconds, recording=recording.to_native()) def set_time_nanos(timeline: str, nanos: int, recording: RecordingStream | None = None) -> None: @@ -104,7 +104,7 @@ def set_time_nanos(timeline: str, nanos: int, recording: RecordingStream | None """ - bindings.set_time_nanos(timeline, nanos, recording=RecordingStream.to_native(recording)) + bindings.set_time_nanos(timeline, nanos, recording=recording.to_native()) def disable_timeline(timeline: str, recording: RecordingStream | None = None) -> None: @@ -122,7 +122,7 @@ def disable_timeline(timeline: str, recording: RecordingStream | None = None) -> """ - bindings.disable_timeline(timeline, recording=RecordingStream.to_native(recording)) + bindings.disable_timeline(timeline, recording=recording.to_native()) def reset_time(recording: RecordingStream | None = None) -> None: @@ -143,4 +143,4 @@ def reset_time(recording: RecordingStream | None = None) -> None: """ - bindings.reset_time(recording=RecordingStream.to_native(recording)) + bindings.reset_time(recording=recording.to_native()) diff --git a/scripts/lint.py b/scripts/lint.py index 13cc8a995885..ea87b7e0c1ea 100755 --- a/scripts/lint.py +++ b/scripts/lint.py @@ -173,8 +173,13 @@ def lint_line( if re.search(r"\b3d\b", line): return "we prefer '3D' over '3d'" - if "recording=rec" in line: - return "you must cast the RecordingStream first: `recording=RecordingStream.to_native(recording)" + if ( + "recording=rec" in line + and "recording=rec.to_native()" not in line + and "recording=recording.to_native()" not in line + and "rr.notebook_show" not in line + ): + return "you must cast the RecordingStream first: `recording=recording.to_native()" if "FIXME" in line: return "we prefer TODO over FIXME" @@ -1232,24 +1237,25 @@ def main() -> None: "./rerun_cpp/docs/html", "./rerun_cpp/src/rerun/c/arrow_c_data_interface.h", # Not our code "./rerun_cpp/src/rerun/third_party/cxxopts.hpp", # vendored + "./rerun_js/node_modules", + "./rerun_js/web-viewer-react/node_modules", + "./rerun_js/web-viewer/index.js", + "./rerun_js/web-viewer/inlined.js", + "./rerun_js/web-viewer/node_modules", + "./rerun_js/web-viewer/re_viewer.js", + "./rerun_js/web-viewer/re_viewer_bg.js", # auto-generated by wasm_bindgen + "./rerun_notebook/node_modules", + "./rerun_notebook/src/rerun_notebook/static", "./rerun_py/.pytest_cache/", "./rerun_py/site/", # is in `.gitignore` which this script doesn't fully respect "./run_wasm/README.md", # Has a "2d" lowercase example in a code snippet "./scripts/lint.py", # we contain all the patterns we are linting against "./scripts/zombie_todos.py", + "./tests/python/gil_stress/main.py", "./tests/python/release_checklist/main.py", "./web_viewer/re_viewer.js", # auto-generated by wasm_bindgen # JS dependencies: - "./rerun_notebook/node_modules", - "./rerun_js/node_modules", # JS files generated during build: - "./rerun_notebook/src/rerun_notebook/static", - "./rerun_js/web-viewer-react/node_modules", - "./rerun_js/web-viewer/index.js", - "./rerun_js/web-viewer/inlined.js", - "./rerun_js/web-viewer/node_modules", - "./rerun_js/web-viewer/re_viewer_bg.js", # auto-generated by wasm_bindgen - "./rerun_js/web-viewer/re_viewer.js", ) should_ignore = parse_gitignore(".gitignore") # TODO(#6730): parse all .gitignore files, not just top-level From 49eb8b3f73ace450b7edc194d7fcd89a34baecb8 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 19 Nov 2024 18:08:56 +0100 Subject: [PATCH 5/9] more of that --- rerun_py/rerun_sdk/rerun/_log.py | 9 +++---- rerun_py/rerun_sdk/rerun/_send_columns.py | 2 +- rerun_py/rerun_sdk/rerun/memory.py | 6 ++++- rerun_py/rerun_sdk/rerun/recording_stream.py | 10 ++++---- rerun_py/rerun_sdk/rerun/sinks.py | 26 ++++++++++++++----- rerun_py/rerun_sdk/rerun/time.py | 27 ++++++++++++++++---- 6 files changed, 56 insertions(+), 24 deletions(-) diff --git a/rerun_py/rerun_sdk/rerun/_log.py b/rerun_py/rerun_sdk/rerun/_log.py index 540e9567aa51..f316b45371aa 100644 --- a/rerun_py/rerun_sdk/rerun/_log.py +++ b/rerun_py/rerun_sdk/rerun/_log.py @@ -253,9 +253,6 @@ def log_components( ) static = True - # Convert to a native recording - recording = recording.to_native() - instanced: dict[str, pa.Array] = {} components = list(components) @@ -294,7 +291,7 @@ def log_components( entity_path, components=instanced, static_=static, - recording=recording.to_native(), + recording=recording.to_native() if recording is not None else None, ) @@ -358,7 +355,7 @@ def log_file_from_path( Path(file_path), entity_path_prefix=entity_path_prefix, static_=static, - recording=recording.to_native(), + recording=recording.to_native() if recording is not None else None, ) @@ -419,7 +416,7 @@ def log_file_from_contents( file_contents, entity_path_prefix=entity_path_prefix, static_=static, - recording=recording.to_native(), + recording=recording.to_native() if recording is not None else None, ) diff --git a/rerun_py/rerun_sdk/rerun/_send_columns.py b/rerun_py/rerun_sdk/rerun/_send_columns.py index 1cf8726060e6..4b9917e9d26b 100644 --- a/rerun_py/rerun_sdk/rerun/_send_columns.py +++ b/rerun_py/rerun_sdk/rerun/_send_columns.py @@ -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.to_native(), + recording=recording.to_native() if recording is not None else None, ) diff --git a/rerun_py/rerun_sdk/rerun/memory.py b/rerun_py/rerun_sdk/rerun/memory.py index 16eec0e8ad2c..613322504f0a 100644 --- a/rerun_py/rerun_sdk/rerun/memory.py +++ b/rerun_py/rerun_sdk/rerun/memory.py @@ -28,7 +28,11 @@ def memory_recording(recording: RecordingStream | None = None) -> MemoryRecordin """ - return MemoryRecording(bindings.memory_recording(recording=recording.to_native())) + return MemoryRecording( + bindings.memory_recording( + recording=recording.to_native() if recording is not None else None, + ) + ) class MemoryRecording: diff --git a/rerun_py/rerun_sdk/rerun/recording_stream.py b/rerun_py/rerun_sdk/rerun/recording_stream.py index 4522e16d7642..84e570e73bbc 100644 --- a/rerun_py/rerun_sdk/rerun/recording_stream.py +++ b/rerun_py/rerun_sdk/rerun/recording_stream.py @@ -307,7 +307,7 @@ def binary_stream(recording: RecordingStream | None = None) -> BinaryStream: An object that can be used to flush or read the data. """ - return BinaryStream(bindings.binary_stream(recording=recording.to_native())) + return BinaryStream(bindings.binary_stream(recording=recording.to_native() if recording is not None else None)) class BinaryStream: @@ -380,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=recording.to_native()) # 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( @@ -402,7 +402,7 @@ def get_application_id( The application ID that this recording is associated with. """ - app_id = bindings.get_application_id(recording=recording.to_native()) + 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 @@ -434,7 +434,7 @@ def get_recording_id( The recording ID that this recording is logging to. """ - rec_id = bindings.get_recording_id(recording=recording.to_native()) + 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 @@ -467,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=recording.to_native()) + 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 diff --git a/rerun_py/rerun_sdk/rerun/sinks.py b/rerun_py/rerun_sdk/rerun/sinks.py index df223da6d4be..5baf23a38a9b 100644 --- a/rerun_py/rerun_sdk/rerun/sinks.py +++ b/rerun_py/rerun_sdk/rerun/sinks.py @@ -131,7 +131,7 @@ def connect_tcp( addr=addr, flush_timeout_sec=flush_timeout_sec, default_blueprint=blueprint_storage, - recording=recording.to_native(), + recording=recording.to_native() if recording is not None else None, ) @@ -180,7 +180,11 @@ def save( application_id=application_id, blueprint=default_blueprint ).storage - bindings.save(path=str(path), default_blueprint=blueprint_storage, recording=recording.to_native()) + 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: @@ -225,7 +229,10 @@ def stdout(default_blueprint: BlueprintLike | None = None, recording: RecordingS application_id=application_id, blueprint=default_blueprint ).storage - bindings.stdout(default_blueprint=blueprint_storage, recording=recording.to_native()) + bindings.stdout( + default_blueprint=blueprint_storage, + recording=recording.to_native() if recording is not None else None, + ) def disconnect(recording: RecordingStream | None = None) -> None: @@ -244,7 +251,9 @@ def disconnect(recording: RecordingStream | None = None) -> None: """ - bindings.disconnect(recording=recording.to_native()) + bindings.disconnect( + recording=recording.to_native() if recording is not None else None, + ) @deprecated( @@ -382,7 +391,7 @@ def serve_web( ws_port, server_memory_limit=server_memory_limit, default_blueprint=blueprint_storage, - recording=recording.to_native(), + recording=recording.to_native() if recording is not None else None, ) @@ -423,7 +432,12 @@ def send_blueprint( blueprint_storage = create_in_memory_blueprint(application_id=application_id, blueprint=blueprint).storage - bindings.send_blueprint(blueprint_storage, make_active, make_default, recording=recording.to_native()) + bindings.send_blueprint( + blueprint_storage, + make_active, + make_default, + recording=recording.to_native() if recording is not None else None, + ) def spawn( diff --git a/rerun_py/rerun_sdk/rerun/time.py b/rerun_py/rerun_sdk/rerun/time.py index d1cc52e8db50..91db732759b8 100644 --- a/rerun_py/rerun_sdk/rerun/time.py +++ b/rerun_py/rerun_sdk/rerun/time.py @@ -32,7 +32,11 @@ def set_time_sequence(timeline: str, sequence: int, recording: RecordingStream | See also: [`rerun.init`][], [`rerun.set_global_data_recording`][]. """ - bindings.set_time_sequence(timeline, sequence, recording=recording.to_native()) + bindings.set_time_sequence( + timeline, + sequence, + recording=recording.to_native() if recording is not None else None, + ) def set_time_seconds(timeline: str, seconds: float, recording: RecordingStream | None = None) -> None: @@ -68,7 +72,11 @@ def set_time_seconds(timeline: str, seconds: float, recording: RecordingStream | """ - bindings.set_time_seconds(timeline, seconds, recording=recording.to_native()) + bindings.set_time_seconds( + timeline, + seconds, + recording=recording.to_native() if recording is not None else None, + ) def set_time_nanos(timeline: str, nanos: int, recording: RecordingStream | None = None) -> None: @@ -104,7 +112,11 @@ def set_time_nanos(timeline: str, nanos: int, recording: RecordingStream | None """ - bindings.set_time_nanos(timeline, nanos, recording=recording.to_native()) + bindings.set_time_nanos( + timeline, + nanos, + recording=recording.to_native() if recording is not None else None, + ) def disable_timeline(timeline: str, recording: RecordingStream | None = None) -> None: @@ -122,7 +134,10 @@ def disable_timeline(timeline: str, recording: RecordingStream | None = None) -> """ - bindings.disable_timeline(timeline, recording=recording.to_native()) + bindings.disable_timeline( + timeline, + recording=recording.to_native() if recording is not None else None, + ) def reset_time(recording: RecordingStream | None = None) -> None: @@ -143,4 +158,6 @@ def reset_time(recording: RecordingStream | None = None) -> None: """ - bindings.reset_time(recording=recording.to_native()) + bindings.reset_time( + recording=recording.to_native() if recording is not None else None, + ) From dfe87d100271dd8a8515f81d18946c39be14c49d Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Wed, 20 Nov 2024 10:15:21 +0100 Subject: [PATCH 6/9] add test --- docs/snippets/all/concepts/explicit_recording.py | 12 ++++++++++++ docs/snippets/snippets.toml | 4 ++++ 2 files changed, 16 insertions(+) create mode 100644 docs/snippets/all/concepts/explicit_recording.py diff --git a/docs/snippets/all/concepts/explicit_recording.py b/docs/snippets/all/concepts/explicit_recording.py new file mode 100644 index 000000000000..38007ee24c87 --- /dev/null +++ b/docs/snippets/all/concepts/explicit_recording.py @@ -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) diff --git a/docs/snippets/snippets.toml b/docs/snippets/snippets.toml index 3561ae49e49e..ae13ed5f35d3 100644 --- a/docs/snippets/snippets.toml +++ b/docs/snippets/snippets.toml @@ -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", From 4e4a70224dc29af9614ea56de60a62bc56fcbf8c Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Wed, 20 Nov 2024 10:22:47 +0100 Subject: [PATCH 7/9] also this --- scripts/lint.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/lint.py b/scripts/lint.py index ea87b7e0c1ea..531d16101f6b 100755 --- a/scripts/lint.py +++ b/scripts/lint.py @@ -177,6 +177,7 @@ def lint_line( "recording=rec" in line and "recording=rec.to_native()" not in line and "recording=recording.to_native()" not in line + and "rr.log" not in line and "rr.notebook_show" not in line ): return "you must cast the RecordingStream first: `recording=recording.to_native()" From 007af5b9429c65cc5b04a11f87804b3d02cf353f Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Wed, 20 Nov 2024 10:43:34 +0100 Subject: [PATCH 8/9] the definition itself was broken?? --- rerun_py/rerun_sdk/rerun/recording_stream.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rerun_py/rerun_sdk/rerun/recording_stream.py b/rerun_py/rerun_sdk/rerun/recording_stream.py index 84e570e73bbc..41b891bc9917 100644 --- a/rerun_py/rerun_sdk/rerun/recording_stream.py +++ b/rerun_py/rerun_sdk/rerun/recording_stream.py @@ -513,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. @@ -523,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=recording.to_native()) + 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 From aea0d460ae8a5a868b267e5dc7996c27a20b5960 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Wed, 20 Nov 2024 10:51:36 +0100 Subject: [PATCH 9/9] smarter lint escape --- scripts/lint.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/lint.py b/scripts/lint.py index 531d16101f6b..63c83d382fe4 100755 --- a/scripts/lint.py +++ b/scripts/lint.py @@ -175,10 +175,9 @@ def lint_line( if ( "recording=rec" in line + and "rr." not in line and "recording=rec.to_native()" not in line and "recording=recording.to_native()" not in line - and "rr.log" not in line - and "rr.notebook_show" not in line ): return "you must cast the RecordingStream first: `recording=recording.to_native()"