Skip to content

Commit

Permalink
Codegen/IDL 7: handwritten Python tests and extensions for Points2D (
Browse files Browse the repository at this point in the history
…#2410)

Implements all the necessary Python extensions so that the UX of the
`Points2D` archetype is --dare I say-- _exquisite_.

Also adds a whole bunch of tests for all of these extensions because,
well.. Python...

Might be easier to review commit by commit 🤷 

---
 
Codegen/IDL PR series:
- #2362
- #2363
- #2369
- #2370 
- #2374 
- #2375 
- #2410
- #2432

---------

Co-authored-by: Andreas Reich <[email protected]>
  • Loading branch information
teh-cmc and Wumpf authored Jun 14, 2023
1 parent 0b349d3 commit 9393da6
Show file tree
Hide file tree
Showing 26 changed files with 578 additions and 48 deletions.
25 changes: 23 additions & 2 deletions crates/re_types/build.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Generates Rust & Python code from flatbuffers definitions.
use std::path::PathBuf;

use xshell::{cmd, Shell};

use re_build_tools::{
Expand Down Expand Up @@ -84,6 +86,15 @@ fn main() {
"./definitions/rerun/archetypes.fbs",
);

let pyproject_path = PathBuf::from(PYTHON_OUTPUT_DIR_PATH)
.parent()
.unwrap()
.parent()
.unwrap()
.join("pyproject.toml")
.to_string_lossy()
.to_string();

// NOTE: This requires both `black` and `ruff` to be in $PATH, but only for contributors,
// not end users.
// Even for contributors, `black` and `ruff` won't be needed unless they edit some of the
Expand All @@ -96,15 +107,25 @@ fn main() {
// the build.
//
// The CI will catch the unformatted files at PR time and complain appropriately anyhow.
cmd!(sh, "black {PYTHON_OUTPUT_DIR_PATH}").run().ok();
cmd!(
sh,
"black --config {pyproject_path} {PYTHON_OUTPUT_DIR_PATH}"
)
.run()
.ok();

// NOTE: We're purposefully ignoring the error here.
//
// If the user doesn't have `ruff` in their $PATH, there's still no good reason to fail
// the build.
//
// The CI will catch the unformatted files at PR time and complain appropriately anyhow.
cmd!(sh, "ruff --fix {PYTHON_OUTPUT_DIR_PATH}").run().ok();
cmd!(
sh,
"ruff --config {pyproject_path} --fix {PYTHON_OUTPUT_DIR_PATH}"
)
.run()
.ok();

write_versioning_hash(SOURCE_HASH_PATH, new_hash);
}
4 changes: 2 additions & 2 deletions crates/re_types/definitions/rerun/components/class_id.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ namespace rerun.components;
/// \rs Used to look up a `crate::components::ClassDescription` within the `crate::components::AnnotationContext`.
struct ClassId (
"attr.arrow.transparent",
"attr.python.aliases": "float",
"attr.python.array_aliases": "npt.NDArray[np.uint8], npt.NDArray[np.uint16], npt.NDArray[np.uint32]",
"attr.python.aliases": "int",
"attr.python.array_aliases": "npt.NDArray[np.uint8], npt.NDArray[np.uint16], npt.NDArray[np.uint32], npt.NDArray[np.uint64]",
"attr.rust.derive": "Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash",
"attr.rust.tuple_struct",
order: 100
Expand Down
8 changes: 6 additions & 2 deletions crates/re_types/definitions/rerun/components/color.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ namespace rerun.components;
// ---

/// An RGBA color tuple with unmultiplied/separate alpha, in sRGB gamma space with linear alpha.
///
/// \py Float colors are assumed to be in 0-1 gamma sRGB space.
/// \py All other colors are assumed to be in 0-255 gamma sRGB space.
/// \py If there is an alpha, we assume it is in linear space, and separate (NOT pre-multiplied).
struct Color (
"attr.arrow.transparent",
"attr.python.aliases": "Sequence[int], Sequence[float], npt.NDArray[np.uint8], npt.NDArray[np.float32], npt.NDArray[np.float64]",
"attr.python.array_aliases": "Sequence[int], Sequence[float], npt.NDArray[np.uint8], npt.NDArray[np.float32], npt.NDArray[np.float64]",
"attr.python.aliases": "int, npt.NDArray[np.uint8], npt.NDArray[np.uint32], npt.NDArray[np.float32], npt.NDArray[np.float64]",
"attr.python.array_aliases": "Sequence[int], npt.NDArray[np.uint8], npt.NDArray[np.uint32], npt.NDArray[np.float32], npt.NDArray[np.float64]",
"attr.rust.derive": "Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, bytemuck::Pod, bytemuck::Zeroable",
"attr.rust.repr": "transparent",
"attr.rust.tuple_struct",
Expand Down
2 changes: 1 addition & 1 deletion crates/re_types/source_hash.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# This is a sha256 hash for all direct and indirect dependencies of this crate's build script.
# It can be safely removed at anytime to force the build script to run again.
# Check out build.rs to see how it's computed.
2d611d0d686a6314da94b1797472e7690413465d16a611aae3560a847668d53d
184f79acfe7c0608f9ec2bb5f6a6783dc2270f5cae0edb147ad08ad4ee89c825
12 changes: 6 additions & 6 deletions crates/re_types_builder/src/codegen/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ fn quote_str_repr_from_obj(obj: &Object) -> String {

unindent::unindent(
r#"
def __str__(self):
def __str__(self) -> str:
s = f"rr.{type(self).__name__}(\n"
from dataclasses import fields
Expand All @@ -482,7 +482,7 @@ fn quote_str_repr_from_obj(obj: &Object) -> String {
return s
def __repr__(self):
def __repr__(self) -> str:
return str(self)
"#,
Expand Down Expand Up @@ -510,7 +510,7 @@ fn quote_array_method_from_obj(objects: &Objects, obj: &Object) -> String {
let field_name = &obj.fields[0].name;
unindent::unindent(&format!(
"
def __array__(self):
def __array__(self) -> npt.ArrayLike:
return np.asarray(self.{field_name})
",
))
Expand All @@ -535,7 +535,7 @@ fn quote_str_method_from_obj(objects: &Objects, obj: &Object) -> String {
let field_name = &obj.fields[0].name;
unindent::unindent(&format!(
"
def __str__(self):
def __str__(self) -> str:
return self.{field_name}
",
))
Expand Down Expand Up @@ -771,7 +771,7 @@ fn quote_arrow_support_from_obj(arrow_registry: &ArrowRegistry, obj: &Object) ->
from .{pkg}_ext import {many}Ext # noqa: E402
class {arrow}(pa.ExtensionType):
class {arrow}(pa.ExtensionType): # type: ignore[misc]
def __init__(self: type[pa.ExtensionType]) -> None:
pa.ExtensionType.__init__(
self, {datatype}, "{fqname}"
Expand All @@ -795,7 +795,7 @@ fn quote_arrow_support_from_obj(arrow_registry: &ArrowRegistry, obj: &Object) ->
class {many}(pa.ExtensionArray, {many}Ext): # type: ignore[misc]
@staticmethod
def from_similar(data: {many_aliases} | None):
def from_similar(data: {many_aliases} | None) -> pa.Array:
if data is None:
return {arrow}().wrap_array(pa.array([], type={arrow}().storage_type))
else:
Expand Down
2 changes: 1 addition & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ py-lint:
# Run fast unittests
py-test:
python -m pytest rerun_py/tests/unit/
python -m pytest -vv rerun_py/tests/unit/

# Serve the python docs locally
py-docs-serve:
Expand Down
4 changes: 2 additions & 2 deletions rerun_py/rerun_sdk/rerun2/archetypes/points2d.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Points2D:
Unique identifiers for each individual point in the batch.
"""

def __str__(self):
def __str__(self) -> str:
s = f"rr.{type(self).__name__}(\n"

from dataclasses import fields
Expand All @@ -86,7 +86,7 @@ def __str__(self):

return s

def __repr__(self):
def __repr__(self) -> str:
return str(self)

def __init__(
Expand Down
15 changes: 10 additions & 5 deletions rerun_py/rerun_sdk/rerun2/components/class_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,19 @@ class ClassId:

id: int

def __array__(self):
def __array__(self) -> npt.ArrayLike:
return np.asarray(self.id)


ClassIdLike = Union[ClassId, float]
ClassIdLike = Union[ClassId, int]

ClassIdArrayLike = Union[
ClassIdLike, Sequence[ClassIdLike], npt.NDArray[np.uint8], npt.NDArray[np.uint16], npt.NDArray[np.uint32]
ClassIdLike,
Sequence[ClassIdLike],
npt.NDArray[np.uint8],
npt.NDArray[np.uint16],
npt.NDArray[np.uint32],
npt.NDArray[np.uint64],
]


Expand All @@ -34,7 +39,7 @@ def __array__(self):
from rerun2.components.class_id_ext import ClassIdArrayExt # noqa: E402


class ClassIdType(pa.ExtensionType):
class ClassIdType(pa.ExtensionType): # type: ignore[misc]
def __init__(self: type[pa.ExtensionType]) -> None:
pa.ExtensionType.__init__(self, pa.uint16(), "rerun.components.ClassId")

Expand All @@ -58,7 +63,7 @@ def __arrow_ext_class__(self: type[pa.ExtensionType]) -> type[pa.ExtensionArray]

class ClassIdArray(pa.ExtensionArray, ClassIdArrayExt): # type: ignore[misc]
@staticmethod
def from_similar(data: ClassIdArrayLike | None):
def from_similar(data: ClassIdArrayLike | None) -> pa.Array:
if data is None:
return ClassIdType().wrap_array(pa.array([], type=ClassIdType().storage_type))
else:
Expand Down
21 changes: 21 additions & 0 deletions rerun_py/rerun_sdk/rerun2/components/class_id_ext.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from __future__ import annotations

__all__ = ["ClassIdArrayExt"]

from typing import Any, Sequence

import numpy as np
import pyarrow as pa


class ClassIdArrayExt:
@staticmethod
def _from_similar(
data: Any | None, *, mono: type, mono_aliases: Any, many: type, many_aliases: Any, arrow: type
) -> pa.Array:
if isinstance(data, Sequence) and (len(data) > 0 and isinstance(data[0], mono)):
array = np.asarray([class_id.id for class_id in data], np.uint16)
else:
array = np.asarray(data, dtype=np.uint16).flatten()

return arrow().wrap_array(pa.array(array, type=arrow().storage_type))
18 changes: 12 additions & 6 deletions rerun_py/rerun_sdk/rerun2/components/color.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,30 @@

@dataclass
class Color:
"""An RGBA color tuple with unmultiplied/separate alpha, in sRGB gamma space with linear alpha."""
"""
An RGBA color tuple with unmultiplied/separate alpha, in sRGB gamma space with linear alpha.
Float colors are assumed to be in 0-1 gamma sRGB space.
All other colors are assumed to be in 0-255 gamma sRGB space.
If there is an alpha, we assume it is in linear space, and separate (NOT pre-multiplied).
"""

rgba: int

def __array__(self):
def __array__(self) -> npt.ArrayLike:
return np.asarray(self.rgba)


ColorLike = Union[
Color, Sequence[int], Sequence[float], npt.NDArray[np.uint8], npt.NDArray[np.float32], npt.NDArray[np.float64]
Color, int, npt.NDArray[np.uint8], npt.NDArray[np.uint32], npt.NDArray[np.float32], npt.NDArray[np.float64]
]

ColorArrayLike = Union[
ColorLike,
Sequence[ColorLike],
Sequence[int],
Sequence[float],
npt.NDArray[np.uint8],
npt.NDArray[np.uint32],
npt.NDArray[np.float32],
npt.NDArray[np.float64],
]
Expand All @@ -42,7 +48,7 @@ def __array__(self):
from rerun2.components.color_ext import ColorArrayExt # noqa: E402


class ColorType(pa.ExtensionType):
class ColorType(pa.ExtensionType): # type: ignore[misc]
def __init__(self: type[pa.ExtensionType]) -> None:
pa.ExtensionType.__init__(self, pa.uint32(), "rerun.components.Color")

Expand All @@ -66,7 +72,7 @@ def __arrow_ext_class__(self: type[pa.ExtensionType]) -> type[pa.ExtensionArray]

class ColorArray(pa.ExtensionArray, ColorArrayExt): # type: ignore[misc]
@staticmethod
def from_similar(data: ColorArrayLike | None):
def from_similar(data: ColorArrayLike | None) -> pa.Array:
if data is None:
return ColorType().wrap_array(pa.array([], type=ColorType().storage_type))
else:
Expand Down
42 changes: 42 additions & 0 deletions rerun_py/rerun_sdk/rerun2/components/color_ext.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from __future__ import annotations

__all__ = ["ColorArrayExt"]

from typing import Any, Sequence

import numpy as np
import pyarrow as pa
from rerun.color_conversion import u8_array_to_rgba


class ColorArrayExt:
@staticmethod
def _from_similar(
data: Any | None, *, mono: type, mono_aliases: Any, many: type, many_aliases: Any, arrow: type
) -> pa.Array:
"""
Normalize flexible colors arrays.
Float colors are assumed to be in 0-1 gamma sRGB space.
All other colors are assumed to be in 0-255 gamma sRGB space.
If there is an alpha, we assume it is in linear space, and separate (NOT pre-multiplied).
"""
if isinstance(data, Sequence) and len(data) == 0:
array = np.array([], np.uint32)
elif isinstance(data, Sequence) and (len(data) > 0 and isinstance(data[0], mono)):
array = np.asarray([color.rgba for color in data], np.uint32)
elif isinstance(data, Sequence) and (len(data) > 0 and isinstance(data[0], int)):
array = np.asarray(data, np.uint32)
else:
array = np.asarray(data)
# Rust expects colors in 0-255 uint8
if array.dtype.type in [np.float32, np.float64]:
# Assume gamma-space colors
array = u8_array_to_rgba(np.asarray(np.round(np.asarray(data).reshape((-1, 4)) * 255.0), np.uint8))
elif array.dtype.type == np.uint32:
array = np.asarray(data).flatten()
else:
array = u8_array_to_rgba(np.asarray(data, dtype=np.uint8).reshape((-1, 4)))

return arrow().wrap_array(pa.array(array, type=arrow().storage_type))
6 changes: 3 additions & 3 deletions rerun_py/rerun_sdk/rerun2/components/draw_order.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class DrawOrder:

value: float

def __array__(self):
def __array__(self) -> npt.ArrayLike:
return np.asarray(self.value)


Expand All @@ -40,7 +40,7 @@ def __array__(self):
from rerun2.components.draw_order_ext import DrawOrderArrayExt # noqa: E402


class DrawOrderType(pa.ExtensionType):
class DrawOrderType(pa.ExtensionType): # type: ignore[misc]
def __init__(self: type[pa.ExtensionType]) -> None:
pa.ExtensionType.__init__(self, pa.float32(), "rerun.components.DrawOrder")

Expand All @@ -64,7 +64,7 @@ def __arrow_ext_class__(self: type[pa.ExtensionType]) -> type[pa.ExtensionArray]

class DrawOrderArray(pa.ExtensionArray, DrawOrderArrayExt): # type: ignore[misc]
@staticmethod
def from_similar(data: DrawOrderArrayLike | None):
def from_similar(data: DrawOrderArrayLike | None) -> pa.Array:
if data is None:
return DrawOrderType().wrap_array(pa.array([], type=DrawOrderType().storage_type))
else:
Expand Down
21 changes: 21 additions & 0 deletions rerun_py/rerun_sdk/rerun2/components/draw_order_ext.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from __future__ import annotations

__all__ = ["DrawOrderArrayExt"]

from typing import Any, Sequence

import numpy as np
import pyarrow as pa


class DrawOrderArrayExt:
@staticmethod
def _from_similar(
data: Any | None, *, mono: type, mono_aliases: Any, many: type, many_aliases: Any, arrow: type
) -> pa.Array:
if isinstance(data, Sequence) and (len(data) > 0 and isinstance(data[0], mono)):
array = np.asarray([draw_order.value for draw_order in data], np.float32)
else:
array = np.require(np.asarray(data), np.float32).flatten()

return arrow().wrap_array(pa.array(array, type=arrow().storage_type))
Loading

1 comment on commit 9393da6

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.25.

Benchmark suite Current: 9393da6 Previous: 0ec609a Ratio
datastore/num_rows=1000/num_instances=1000/packed=false/insert/default 6182181 ns/iter (± 69854) 2883912 ns/iter (± 28153) 2.14
datastore/num_rows=1000/num_instances=1000/packed=false/latest_at/default 408 ns/iter (± 2) 306 ns/iter (± 1) 1.33
datastore/num_rows=1000/num_instances=1000/packed=false/latest_at_missing/primary/default 298 ns/iter (± 0) 222 ns/iter (± 3) 1.34
datastore/num_rows=1000/num_instances=1000/packed=false/latest_at_missing/secondaries/default 456 ns/iter (± 0) 336 ns/iter (± 3) 1.36
datastore/num_rows=1000/num_instances=1000/packed=false/range/default 6745655 ns/iter (± 265327) 2930889 ns/iter (± 39330) 2.30
datastore/num_rows=1000/num_instances=1000/gc/default 2923380 ns/iter (± 47395) 1731589 ns/iter (± 26107) 1.69
mono_points_arrow/generate_message_bundles 41705506 ns/iter (± 1047502) 28157682 ns/iter (± 789944) 1.48
mono_points_arrow/generate_messages 176002109 ns/iter (± 1383778) 137173359 ns/iter (± 1238292) 1.28
mono_points_arrow/encode_log_msg 189465692 ns/iter (± 1402516) 136848583 ns/iter (± 1202825) 1.38
mono_points_arrow/encode_total 395051054 ns/iter (± 5398731) 312474889 ns/iter (± 1771228) 1.26
mono_points_arrow/decode_message_bundles 91524862 ns/iter (± 1074152) 57064034 ns/iter (± 724692) 1.60
mono_points_arrow_batched/generate_message_bundles 36408281 ns/iter (± 756893) 18653807 ns/iter (± 141047) 1.95
mono_points_arrow_batched/generate_messages 12378844 ns/iter (± 805188) 3542612 ns/iter (± 34238) 3.49
mono_points_arrow_batched/encode_total 50138704 ns/iter (± 1737361) 23717322 ns/iter (± 324048) 2.11
mono_points_arrow_batched/decode_log_msg 564645 ns/iter (± 9041) 305644 ns/iter (± 2101) 1.85
mono_points_arrow_batched/decode_message_bundles 14083774 ns/iter (± 371830) 7322670 ns/iter (± 7163) 1.92
mono_points_arrow_batched/decode_total 15026012 ns/iter (± 468225) 7750874 ns/iter (± 9891) 1.94
batch_points_arrow/decode_log_msg 78393 ns/iter (± 1404) 48811 ns/iter (± 115) 1.61
batch_points_arrow/decode_total 86696 ns/iter (± 1924) 51283 ns/iter (± 306) 1.69
arrow_mono_points/insert 3489338759 ns/iter (± 48659989) 1778287912 ns/iter (± 4089788) 1.96
arrow_mono_points/query 1849409 ns/iter (± 192078) 938186 ns/iter (± 7208) 1.97

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

Please sign in to comment.