Skip to content

Commit

Permalink
Improve edge-cases and warn on ambiguity for Rgba32 datatype (rerun-i…
Browse files Browse the repository at this point in the history
…o#8054)

### What
- Resolves: rerun-io#8035
- Makes it so lists of ints are generally assumed to be packed colors
unless they are length 3 or 4
- Warns when passing a length 3 or 4 list of colors that uses values
outside of the u8 range.
- Preserves the old functionality when explicitly passing an np.array of
the right type.

The previous offending code:
```python
import rerun as rr

rr.init("rerun_example_colorbad", spawn=True)

point = [[0, 0, 0]]
colors = [0x00FF00FF]  # green

for n in range(30):
    rr.log("points3d", rr.Points3D(positions=point * n, colors=colors * n))
```

Now works, but produces a warning:
```
/home/jleibs/colorbad.py:11: RerunWarning: Ambiguous input for colors of length 4. If using 0xRRGGBBAA values, please wrap as np.array with dtype=np.uint32
  rr.log("points3d", rr.Points3D(positions=point * n, colors=colors * n))
```

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/8054?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/8054?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/8054)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
jleibs authored Nov 11, 2024
1 parent 5aaf203 commit 59c5750
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 14 deletions.
31 changes: 30 additions & 1 deletion docs/content/reference/migration/migration-0-20.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ order: 990

## ⚠️ Breaking changes


### `connect` -> `connect_tcp` & `serve` -> `serve_web`

In all SDKs:
Expand All @@ -26,6 +25,36 @@ You can learn more about Rerun's application model and the different servers and

Note that this doesn't affect `re_dataframe`, where this type was already re-exported as `QueryCache`.

### Python `colors` change in behavior for single-dimensional lists

Single-dimensional lists that don't otherwise provide type information are now be assumed to be packed
integers color representations (e.g. `0xRRGGBBAA`), unless the length is exactly 3 or 4.

In the case of single lists of 3 or 4 elements, we continue to allow the common pattern of writing: `colors=[r, g, b]`.

This change primarily impacts a previous feature in which all lists divisible by 4 were assumed to be alternating,
`[r, g, b, a, r, g, b, a, …]`. This feature is still available, but depends on your input explicitly being typed
as a numpy array of `np.uint8`.

If you depend on code that uses a bare python list of alternating colors, such as:
```python
rr.log("my_points", rr.Points3D(…, colors=[r, g, b, a, r, g, b, a, …]))
```
You should wrap your input explicitly in a `np.uint8` typed numpy array:
```python
rr.log("my_points", rr.Points3D(…, colors=np.array([r, g, b, a, r, g, b, a, …], dtype=np.uint8)))
```

Additionally, if you are making use of packed integer colors, it is also advised to add the `np.uint32` type,
as otherwise length-3 or length-4 lists will risk being interpreted incorrectly.
```python
rr.log("my_points", rr.Points3D(…, colors=[0xff0000ff, 0x00ff00ff, 0x0000ffff, …]))
```
becomes
```python
rr.log("my_points", rr.Points3D(…, colors=np.array([0xff0000ff, 0x00ff00ff, 0x0000ffff, …], dtype=np.uint32)))
```

## ❗ Deprecations

Support for Python 3.8 is being deprecated. Python 3.8 is past end-of-life. See: https://devguide.python.org/versions/
Expand Down
50 changes: 37 additions & 13 deletions rerun_py/rerun_sdk/rerun/datatypes/rgba32_ext.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
from __future__ import annotations

import warnings
from typing import TYPE_CHECKING, Sequence

import numpy as np
import numpy.typing as npt
import pyarrow as pa

from rerun.color_conversion import u8_array_to_rgba
from ..color_conversion import u8_array_to_rgba
from ..error_utils import RerunWarning

if TYPE_CHECKING:
from . import Rgba32ArrayLike, Rgba32Like
Expand All @@ -18,8 +20,6 @@ def _numpy_array_to_u32(data: npt.NDArray[np.uint8 | np.float32 | np.float64]) -

if data.dtype.type in [np.float32, np.float64]:
array = u8_array_to_rgba(np.asarray(np.round(np.asarray(data) * 255.0), np.uint8))
elif data.dtype.type == np.uint32:
array = np.asarray(data, dtype=np.uint32).flatten()
else:
array = u8_array_to_rgba(np.asarray(data, dtype=np.uint8))
return array
Expand Down Expand Up @@ -71,21 +71,45 @@ def native_to_pa_array_override(data: Rgba32ArrayLike, data_type: pa.DataType) -
else:
# Try to coerce it to a numpy array
try:
arr = np.asarray(data)
arr = np.asarray(data).squeeze()

# If the array is flat
if len(arr.shape) <= 1:
# And not one of the known types
if arr.dtype not in (np.uint8, np.float32, np.float64, np.uint32):
# And not a length 3 or 4 array:
if arr.size not in (3, 4):
# We assume this is packed ints
arr = arr.astype(np.uint32)
else:
# Otherwise, if all the values are less than 256
if np.max(arr) < 256:
# Then treat it as a single color
arr = arr.reshape((1, -1))
else:
# But if not, then send a warning to the user
warnings.warn(
f"Ambiguous input for colors of length {arr.size}. If using 0xRRGGBBAA values, please wrap as np.array with dtype=np.uint32",
category=RerunWarning,
stacklevel=7,
)
arr = arr.astype(np.uint32)

elif arr.dtype != np.uint32:
if len(arr.shape) <= 1:
if arr.size > 4:
# multiple RGBA colors
arr = arr.reshape((-1, 4))
else:
# a single color
arr = arr.reshape((1, -1))

if arr.dtype == np.uint32:
# these are already packed values
int_array = arr.flatten()
else:
# these are component values
if len(arr.shape) == 1:
if arr.size > 4:
# multiple RGBA colors
arr = arr.reshape((-1, 4))
else:
# a single color
arr = arr.reshape((1, -1))
int_array = _numpy_array_to_u32(arr)
int_array = _numpy_array_to_u32(arr) # type: ignore[assignment]

except (ValueError, TypeError, IndexError):
# Fallback support
data_list = list(data) # type: ignore[arg-type]
Expand Down
101 changes: 101 additions & 0 deletions rerun_py/tests/unit/test_rgba32.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
from __future__ import annotations

import numpy as np
import pytest
import rerun as rr
from rerun.datatypes import Rgba32ArrayLike, Rgba32Batch
from rerun.error_utils import RerunWarning

CASES: list[tuple[Rgba32ArrayLike, Rgba32ArrayLike]] = [
(
[],
[],
),
(
0x12345678,
[0x12345678],
),
(
[0x12345678],
[0x12345678],
),
(
[[0x12345678]],
[0x12345678],
),
(
[1, 2, 3],
[0x010203FF],
),
(
[[1, 2, 3]],
[0x010203FF],
),
(
[1.0, 1.0, 1.0],
[0xFFFFFFFF],
),
(
[[1.0, 1.0, 1.0]],
[0xFFFFFFFF],
),
(
[1, 2, 3, 4],
[0x01020304],
),
(
[[1, 2, 3, 4]],
[0x01020304],
),
(
[0x11000000, 0x00220000],
[0x11000000, 0x00220000],
),
(
[[1, 2, 3, 4], [5, 6, 7, 8]],
[0x01020304, 0x05060708],
),
(
[1, 2, 3, 4, 5, 6, 7, 8],
[1, 2, 3, 4, 5, 6, 7, 8],
),
(
np.array([1, 2, 3, 4, 5, 6, 7, 8], dtype=np.uint8),
[0x01020304, 0x05060708],
),
(
[[1, 2, 3, 4], [5, 6, 7, 8], [9, 10, 11, 12]],
[0x01020304, 0x05060708, 0x090A0B0C],
),
(
np.array([0x11000000, 0x00220000, 0x00003300], dtype=np.uint32),
[0x11000000, 0x00220000, 0x00003300],
),
]


def test_rgba() -> None:
for input, expected in CASES:
data = Rgba32Batch(input)
assert data.as_arrow_array().to_pylist() == expected


AMBIGUOUS_CASES: list[tuple[Rgba32ArrayLike, Rgba32ArrayLike]] = [
(
[0x11000000, 0x00220000, 0x00003300],
[0x11000000, 0x00220000, 0x00003300],
),
(
[0x11000000, 0x00220000, 0x00003300, 0x00000044],
[0x11000000, 0x00220000, 0x00003300, 0x00000044],
),
]


def test_ambiguous_rgba() -> None:
rr.init("rerun_example_ambiguous_rgba", strict=False)
for input, expected in AMBIGUOUS_CASES:
with pytest.warns(RerunWarning) as warnings:
data = Rgba32Batch(input)
assert data.as_arrow_array().to_pylist() == expected
assert len(warnings) == 1

0 comments on commit 59c5750

Please sign in to comment.