Skip to content

Commit

Permalink
Python SDK: Add strict mode (#1477)
Browse files Browse the repository at this point in the history
* Python SDK: Add a decorator to all out logging function to early-out

This replaces explicit checks with a decorator.

This decorator declaration preserves type information
in Python 3.8 in my VSCode, which was a problem with previous attempts.

In a follow-up PR we can add a try-catch to the decorator to prevent
errors from bubbling up to the user.

* Catch and log exceptions in log_decorator

* Replace a few _send_warning with raise

* Make strict-mode opt-in

* Clean up scripts/lint.py

* emil should learn to type and and spellcheck

Co-authored-by: Andreas Reich <[email protected]>

* can we get automated spell checking of docstrings?

Co-authored-by: Andreas Reich <[email protected]>

* surely there is a spell checker for python docstrings, right?

Co-authored-by: Andreas Reich <[email protected]>

* not misuses - incorrect use

---------

Co-authored-by: Andreas Reich <[email protected]>
  • Loading branch information
emilk and Wumpf authored Mar 2, 2023
1 parent e269138 commit 684a070
Show file tree
Hide file tree
Showing 20 changed files with 140 additions and 80 deletions.
40 changes: 39 additions & 1 deletion rerun_py/rerun_sdk/rerun/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@
]


# If `True`, we raise exceptions on use error (wrong parameter types etc).
# If `False` we catch all errors and log a warning instead.
_strict_mode = False


def rerun_shutdown() -> None:
bindings.shutdown()

Expand Down Expand Up @@ -118,7 +123,7 @@ def set_recording_id(value: str) -> None:
bindings.set_recording_id(value)


def init(application_id: str, spawn: bool = False, default_enabled: bool = True) -> None:
def init(application_id: str, spawn: bool = False, default_enabled: bool = True, strict: bool = False) -> None:
"""
Initialize the Rerun SDK with a user-chosen application id (name).
Expand All @@ -139,8 +144,13 @@ def init(application_id: str, spawn: bool = False, default_enabled: bool = True)
default_enabled
Should Rerun logging be on by default?
Can overridden with the RERUN env-var, e.g. `RERUN=on` or `RERUN=off`.
strict
If `True`, an exceptions is raised on use error (wrong parameter types etc).
If `False`, errors are logged as warnings instead.
"""

_strict_mode = strict
application_path = None

# NOTE: It'd be even nicer to do such thing on the Rust-side so that this little trick would
Expand Down Expand Up @@ -216,6 +226,34 @@ def set_enabled(enabled: bool) -> None:
bindings.set_enabled(enabled)


def strict_mode() -> bool:
"""
Strict mode enabled.
In strict mode, incorrect use of the Rerun API (wrong parameter types etc.)
will result in exception being raised.
When strict mode is on, such problems are instead logged as warnings.
The default is OFF.
"""

return _strict_mode


def set_strict_mode(strict_mode: bool) -> None:
"""
Turn strict mode on/off.
In strict mode, incorrect use of the Rerun API (wrong parameter types etc.)
will result in exception being raised.
When strict mode is off, such problems are instead logged as warnings.
The default is OFF.
"""

_strict_mode = strict_mode


def connect(addr: Optional[str] = None) -> None:
"""
Connect to a remote Rerun Viewer on the given ip:port.
Expand Down
5 changes: 2 additions & 3 deletions rerun_py/rerun_sdk/rerun/log/annotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from rerun import bindings
from rerun.log import Color, _normalize_colors
from rerun.log.log_decorator import log_decorator

__all__ = [
"AnnotationInfo",
Expand Down Expand Up @@ -72,6 +73,7 @@ def coerce_class_descriptor_like(arg: ClassDescriptionLike) -> ClassDescription:
return ClassDescription(info=arg) # type: ignore[arg-type]


@log_decorator
def log_annotation_context(
entity_path: str,
class_descriptions: Union[ClassDescriptionLike, Iterable[ClassDescriptionLike]],
Expand Down Expand Up @@ -114,9 +116,6 @@ def log_annotation_context(
"""

if not bindings.is_enabled():
return

if not isinstance(class_descriptions, Iterable):
class_descriptions = [class_descriptions]

Expand Down
5 changes: 2 additions & 3 deletions rerun_py/rerun_sdk/rerun/log/arrow.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@
from rerun.components.radius import RadiusArray
from rerun.log import _normalize_colors, _normalize_radii
from rerun.log.extension_components import _add_extension_components
from rerun.log.log_decorator import log_decorator

__all__ = [
"log_arrow",
]


@log_decorator
def log_arrow(
entity_path: str,
origin: Optional[npt.ArrayLike],
Expand Down Expand Up @@ -58,9 +60,6 @@ def log_arrow(
"""

if not bindings.is_enabled():
return

instanced: Dict[str, Any] = {}
splats: Dict[str, Any] = {}

Expand Down
5 changes: 2 additions & 3 deletions rerun_py/rerun_sdk/rerun/log/bounding_box.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@
from rerun.components.vec import Vec3DArray
from rerun.log import _normalize_colors, _normalize_ids, _normalize_radii
from rerun.log.extension_components import _add_extension_components
from rerun.log.log_decorator import log_decorator

__all__ = [
"log_obb",
]


@log_decorator
def log_obb(
entity_path: str,
half_size: Optional[npt.ArrayLike],
Expand Down Expand Up @@ -66,9 +68,6 @@ def log_obb(
"""

if not bindings.is_enabled():
return

instanced: Dict[str, Any] = {}
splats: Dict[str, Any] = {}

Expand Down
5 changes: 2 additions & 3 deletions rerun_py/rerun_sdk/rerun/log/camera.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
import numpy.typing as npt

from rerun import bindings
from rerun.log.log_decorator import log_decorator

__all__ = [
"log_pinhole",
]


@log_decorator
def log_pinhole(
entity_path: str,
*,
Expand Down Expand Up @@ -59,9 +61,6 @@ def log_pinhole(
"""

if not bindings.is_enabled():
return

# Transform arrow handling happens inside the python bridge
bindings.log_pinhole(
entity_path,
Expand Down
13 changes: 12 additions & 1 deletion rerun_py/rerun_sdk/rerun/log/error_utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import inspect
import logging

from rerun import bindings
from rerun.log.text import LogLevel, log_text_entry

__all__ = [
Expand All @@ -15,7 +16,17 @@ def _build_warning_context_string(skip_first: int) -> str:


def _send_warning(message: str, depth_to_user_code: int) -> None:
"""Sends a warning about the usage of the Rerun SDK."""
"""
Sends a warning about the usage of the Rerun SDK.
Used for recoverable problems.
You can also use this fo unrecoverable problems,
or raise an exception and let the @log_decorator handle it instead.
"""

if bindings.strict_mode():
raise TypeError(message)

context_descriptor = _build_warning_context_string(skip_first=depth_to_user_code + 2)
warning = f"{message}\n{context_descriptor}"
log_text_entry("rerun", warning, level=LogLevel.WARN)
Expand Down
5 changes: 2 additions & 3 deletions rerun_py/rerun_sdk/rerun/log/extension_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import rerun.log.error_utils
from rerun import bindings
from rerun.components.instance import InstanceArray
from rerun.log.log_decorator import log_decorator

__all__ = [
"_add_extension_components",
Expand Down Expand Up @@ -61,6 +62,7 @@ def _add_extension_components(
instanced[name] = pa_value


@log_decorator
def log_extension_components(
entity_path: str,
ext: Dict[str, Any],
Expand Down Expand Up @@ -109,9 +111,6 @@ def log_extension_components(
"""

if not bindings.is_enabled():
return

identifiers_np = np.array((), dtype="uint64")
if identifiers:
try:
Expand Down
9 changes: 3 additions & 6 deletions rerun_py/rerun_sdk/rerun/log/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import numpy.typing as npt

from rerun import bindings
from rerun.log.log_decorator import log_decorator

__all__ = [
"MeshFormat",
Expand Down Expand Up @@ -39,6 +40,7 @@ class ImageFormat(Enum):
"""JPEG format."""


@log_decorator
def log_mesh_file(
entity_path: str,
mesh_format: MeshFormat,
Expand Down Expand Up @@ -77,9 +79,6 @@ def log_mesh_file(
"""

if not bindings.is_enabled():
return

if transform is None:
transform = np.empty(shape=(0, 0), dtype=np.float32)
else:
Expand All @@ -89,6 +88,7 @@ def log_mesh_file(
bindings.log_mesh_file(entity_path, mesh_format.value, mesh_file, transform, timeless)


@log_decorator
def log_image_file(
entity_path: str,
*,
Expand Down Expand Up @@ -121,9 +121,6 @@ def log_image_file(
"""

if not bindings.is_enabled():
return

img_format = getattr(img_format, "value", None)

# Image file arrow handling happens inside the python bridge
Expand Down
13 changes: 4 additions & 9 deletions rerun_py/rerun_sdk/rerun/log/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from rerun import bindings
from rerun.log.error_utils import _send_warning
from rerun.log.log_decorator import log_decorator
from rerun.log.tensor import Tensor, _log_tensor, _to_numpy

__all__ = [
Expand All @@ -14,6 +15,7 @@
]


@log_decorator
def log_image(
entity_path: str,
image: Tensor,
Expand Down Expand Up @@ -47,9 +49,6 @@ def log_image(
"""

if not bindings.is_enabled():
return

image = _to_numpy(image)

shape = image.shape
Expand Down Expand Up @@ -77,6 +76,7 @@ def log_image(
_log_tensor(entity_path, image, ext=ext, timeless=timeless)


@log_decorator
def log_depth_image(
entity_path: str,
image: Tensor,
Expand Down Expand Up @@ -111,9 +111,6 @@ def log_depth_image(
"""

if not bindings.is_enabled():
return

image = _to_numpy(image)

# TODO(#635): Remove when issue with displaying f64 depth images is fixed.
Expand Down Expand Up @@ -142,6 +139,7 @@ def log_depth_image(
)


@log_decorator
def log_segmentation_image(
entity_path: str,
image: npt.ArrayLike,
Expand Down Expand Up @@ -174,9 +172,6 @@ def log_segmentation_image(
"""

if not bindings.is_enabled():
return

image = np.array(image, copy=False)
if image.dtype not in (np.dtype("uint8"), np.dtype("uint16")):
image = np.require(image, np.uint16)
Expand Down
11 changes: 3 additions & 8 deletions rerun_py/rerun_sdk/rerun/log/lines.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from rerun.components.radius import RadiusArray
from rerun.log import _normalize_colors, _normalize_radii
from rerun.log.extension_components import _add_extension_components
from rerun.log.log_decorator import log_decorator

__all__ = [
"log_path",
Expand All @@ -29,11 +30,10 @@ def log_path(
ext: Optional[Dict[str, Any]] = None,
timeless: bool = False,
) -> None:
if not bindings.is_enabled():
return
log_line_strip(entity_path, positions, stroke_width=stroke_width, color=color, ext=ext, timeless=timeless)


@log_decorator
def log_line_strip(
entity_path: str,
positions: Optional[npt.ArrayLike],
Expand Down Expand Up @@ -73,9 +73,6 @@ def log_line_strip(
"""

if not bindings.is_enabled():
return

if positions is not None:
positions = np.require(positions, dtype="float32")

Expand Down Expand Up @@ -111,6 +108,7 @@ def log_line_strip(
bindings.log_arrow_msg(entity_path, components=instanced, timeless=timeless)


@log_decorator
def log_line_segments(
entity_path: str,
positions: npt.ArrayLike,
Expand Down Expand Up @@ -149,9 +147,6 @@ def log_line_segments(
"""

if not bindings.is_enabled():
return

if positions is None:
positions = np.require([], dtype="float32")
positions = np.require(positions, dtype="float32")
Expand Down
Loading

1 comment on commit 684a070

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust Benchmark

Benchmark suite Current: 684a070 Previous: e269138 Ratio
datastore/insert/batch/rects/insert 552596 ns/iter (± 2517) 567519 ns/iter (± 2076) 0.97
datastore/latest_at/batch/rects/query 1850 ns/iter (± 1) 1845 ns/iter (± 5) 1.00
datastore/latest_at/missing_components/primary 354 ns/iter (± 0) 356 ns/iter (± 0) 0.99
datastore/latest_at/missing_components/secondaries 424 ns/iter (± 0) 424 ns/iter (± 2) 1
datastore/range/batch/rects/query 153209 ns/iter (± 344) 153413 ns/iter (± 338) 1.00
mono_points_arrow/generate_message_bundles 48546958 ns/iter (± 509338) 46588879 ns/iter (± 1693664) 1.04
mono_points_arrow/generate_messages 126227211 ns/iter (± 1150414) 134835762 ns/iter (± 1239523) 0.94
mono_points_arrow/encode_log_msg 154022496 ns/iter (± 1350284) 166489423 ns/iter (± 930285) 0.93
mono_points_arrow/encode_total 328148141 ns/iter (± 1574409) 352950283 ns/iter (± 1518198) 0.93
mono_points_arrow/decode_log_msg 177923468 ns/iter (± 951081) 187049495 ns/iter (± 985910) 0.95
mono_points_arrow/decode_message_bundles 65106130 ns/iter (± 917812) 70503236 ns/iter (± 1155819) 0.92
mono_points_arrow/decode_total 241131322 ns/iter (± 2052348) 255614342 ns/iter (± 1722233) 0.94
batch_points_arrow/generate_message_bundles 332380 ns/iter (± 976) 332788 ns/iter (± 789) 1.00
batch_points_arrow/generate_messages 6285 ns/iter (± 10) 6212 ns/iter (± 26) 1.01
batch_points_arrow/encode_log_msg 354912 ns/iter (± 933) 356270 ns/iter (± 2407) 1.00
batch_points_arrow/encode_total 713272 ns/iter (± 2399) 708418 ns/iter (± 2451) 1.01
batch_points_arrow/decode_log_msg 345328 ns/iter (± 885) 344241 ns/iter (± 922) 1.00
batch_points_arrow/decode_message_bundles 2088 ns/iter (± 4) 2078 ns/iter (± 7) 1.00
batch_points_arrow/decode_total 355143 ns/iter (± 1540) 354099 ns/iter (± 897) 1.00
arrow_mono_points/insert 6059642661 ns/iter (± 278228563) 6825324787 ns/iter (± 33113322) 0.89
arrow_mono_points/query 1711344 ns/iter (± 11509) 1749524 ns/iter (± 11773) 0.98
arrow_batch_points/insert 2661300 ns/iter (± 43139) 2660679 ns/iter (± 14320) 1.00
arrow_batch_points/query 16032 ns/iter (± 26) 16057 ns/iter (± 40) 1.00
arrow_batch_vecs/insert 42228 ns/iter (± 95) 42987 ns/iter (± 465) 0.98
arrow_batch_vecs/query 506305 ns/iter (± 939) 506980 ns/iter (± 1266) 1.00
tuid/Tuid::random 34 ns/iter (± 0) 34 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.