Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 68 additions & 17 deletions python/xy/channels.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,18 @@ def _category_code_dtype(category_count: int) -> type[np.uint8] | type[np.uint32
return np.uint8 if category_count <= MAX_CATEGORIES else np.uint32


def _use_native_fixed_factorizer(arr: np.ndarray) -> bool:
def _factorize_probe(arr: np.ndarray) -> np.ndarray:
"""Return the bounded sample used to select the native factorizer."""
n = len(arr)
if n <= _FACTORIZE_PROBE_ROWS:
return arr
rows = np.linspace(0, n - 1, _FACTORIZE_PROBE_ROWS, dtype=np.intp)
return arr[rows]


def _use_native_fixed_factorizer(
arr: np.ndarray, *, normalized_itemsize: Optional[int] = None
) -> bool:
"""Choose the O(N) hash path unless a bounded global probe says it cannot pay.

The native pass earns its keep by keeping N records out of Python: only the
Expand All @@ -356,21 +367,43 @@ def _use_native_fixed_factorizer(arr: np.ndarray) -> bool:
repeats get scarce while narrow ones hold it until the probe is entirely
distinct. Sampling across the full array keeps the decision independent of N.
"""
n = len(arr)
if n <= _FACTORIZE_PROBE_ROWS:
probe = arr
else:
rows = np.linspace(0, n - 1, _FACTORIZE_PROBE_ROWS, dtype=np.intp)
probe = arr[rows]
probe = _factorize_probe(arr)
distinct = len(np.unique(probe))
if distinct <= _FACTORIZE_NATIVE_MAX_PROBE_CATEGORIES:
return True
near_unique = (
1.0 if arr.dtype.itemsize <= _FACTORIZE_NARROW_ITEMSIZE else _FACTORIZE_NEAR_UNIQUE_RATIO
1.0
if (normalized_itemsize or arr.dtype.itemsize) <= _FACTORIZE_NARROW_ITEMSIZE
else _FACTORIZE_NEAR_UNIQUE_RATIO
)
return distinct < near_unique * len(probe)


def _object_string_width(arr: np.ndarray, *, sample: bool = False) -> Optional[int]:
"""Return the width of safe Python-string values, or ``None``.

Object arrays can contain values with display-label semantics that cannot
be represented by a simple Unicode cast. Restrict this path to exact
Python strings so mixed objects, missing values, and custom ``__str__``
implementations retain the canonical fallback behavior. NUL codepoints
are excluded because NumPy treats them as fixed-width string padding.
"""
if arr.dtype.kind != "O" or arr.size == 0:
return None
values = _factorize_probe(arr) if sample else arr.reshape(-1)
width = 0
for value in values:
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
if type(value) is not str or "\x00" in value:
return None
width = max(width, len(value))
return width


def _normalize_object_strings(arr: np.ndarray, width: int) -> np.ndarray:
"""Normalize validated object-backed strings to fixed-width Unicode."""
return np.asarray(arr, dtype=f"<U{max(width, 1)}")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.


def _factorize_categories(
arr: np.ndarray,
) -> tuple[
Expand All @@ -385,18 +418,36 @@ def _factorize_categories(
not mutually orderable. Chart labels are strings on the client anyway, so
canonicalize to display labels first, sort those labels for deterministic
palettes, and then map each row back to its code. Fixed-width NumPy
strings/bytes/bools can identify equal records in Rust without creating N
Python objects; only their compact unique set crosses the label-policy path.
strings/bytes/bools and safe object-backed Python strings can identify equal
records in Rust without creating N Python objects; only their compact unique
set crosses the label-policy path. Mixed objects, missing values, custom
string semantics, and strings containing NUL codepoints retain the Python
fallback so display-label behavior is unchanged.
"""
if arr.dtype.kind in ("U", "S", "b") and _use_native_fixed_factorizer(arr):
factorizer_arr = arr
if arr.dtype.kind == "O":
# Probe object strings before the full validation and Unicode copy so
# near-unique columns take the existing Python fallback cheaply.
sample_width = _object_string_width(arr, sample=True)
if sample_width is not None and _use_native_fixed_factorizer(
arr, normalized_itemsize=4 * max(sample_width, 1)
):
width = _object_string_width(arr)
if width is not None and _use_native_fixed_factorizer(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: The two native-factorizer gates on an object column both call _use_native_fixed_factorizer(arr, ...) with the same object array, so the bounded probe and its np.unique distinct-count are recomputed identically on the same sample for the sample_width check and the full width check. Since the full-scan width is always >= the sample width, the second gate only re-evaluates the same probe with a different normalized_itemsize ratio. Consider computing the probe once (e.g., sampling inside the caller, or precomputing distinct and the probe length once) and only re-deriving the near_unique ratio for each width, which avoids duplicated work on the hot categorical path this code is explicitly tuned for.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python/xy/channels.py, line 436:

<comment>The two native-factorizer gates on an object column both call `_use_native_fixed_factorizer(arr, ...)` with the same object array, so the bounded probe and its `np.unique` distinct-count are recomputed identically on the same sample for the `sample_width` check and the full `width` check. Since the full-scan width is always >= the sample width, the second gate only re-evaluates the same probe with a different `normalized_itemsize` ratio. Consider computing the probe once (e.g., sampling inside the caller, or precomputing `distinct` and the probe length once) and only re-deriving the `near_unique` ratio for each width, which avoids duplicated work on the hot categorical path this code is explicitly tuned for.</comment>

<file context>
@@ -404,11 +418,25 @@ def _factorize_categories(
+            arr, normalized_itemsize=4 * max(sample_width, 1)
+        ):
+            width = _object_string_width(arr)
+            if width is not None and _use_native_fixed_factorizer(
+                arr, normalized_itemsize=4 * max(width, 1)
+            ):
</file context>

arr, normalized_itemsize=4 * max(width, 1)
):
factorizer_arr = _normalize_object_strings(arr, width)
if factorizer_arr.dtype.kind in ("U", "S", "b") and _use_native_fixed_factorizer(
factorizer_arr
):
compact = (
kernels.factorize_unicode1_u8_counts(arr, MAX_CATEGORIES)
if arr.dtype.kind == "U" and arr.dtype.itemsize == 4
else kernels.factorize_fixed_u8_counts(arr, MAX_CATEGORIES)
kernels.factorize_unicode1_u8_counts(factorizer_arr, MAX_CATEGORIES)
if factorizer_arr.dtype.kind == "U" and factorizer_arr.dtype.itemsize == 4
else kernels.factorize_fixed_u8_counts(factorizer_arr, MAX_CATEGORIES)
)
if compact is not None:
raw_codes, unique_indices, raw_counts = compact
unique_labels = [category_label(value) for value in arr[unique_indices]]
unique_labels = [category_label(value) for value in factorizer_arr[unique_indices]]
categories = sorted(set(unique_labels))
index = {label: i for i, label in enumerate(categories)}
remap = np.fromiter(
Expand All @@ -412,8 +463,8 @@ def _factorize_categories(
counts[index[label]] += count
return categories, raw_codes, counts

raw_codes, unique_indices = kernels.factorize_fixed(arr)
unique_labels = [category_label(value) for value in arr[unique_indices]]
raw_codes, unique_indices = kernels.factorize_fixed(factorizer_arr)
unique_labels = [category_label(value) for value in factorizer_arr[unique_indices]]
categories = sorted(set(unique_labels))
index = {label: i for i, label in enumerate(categories)}
dtype = _category_code_dtype(len(categories))
Expand Down
74 changes: 74 additions & 0 deletions tests/test_custom_ramps_and_palette.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,80 @@ def test_the_literal_color_probe_does_not_materialize_category_columns():
assert channels._literal_color_rgba(column) is None


def test_homogeneous_object_categories_use_native_factorization(monkeypatch):
"""Object strings use the native path without an N-entry label list."""
values = np.array(["group-a", "group-b", "group-a", "group-c"] * 250, dtype=object)
seen: list[object] = []
original = channels.category_label

def record(value: object) -> str:
seen.append(value)
return original(value)

monkeypatch.setattr(channels, "category_label", record)
categories, codes, counts = channels._factorize_categories(values)

assert categories == ["group-a", "group-b", "group-c"]
np.testing.assert_array_equal(codes[:4], [0, 1, 0, 2])
np.testing.assert_array_equal(counts, [500, 250, 250])
assert len(seen) == len(categories)


def test_object_factorization_matches_fixed_width_string_semantics():
"""The object fast path preserves category order, codes, and counts."""
values = np.array(["beta", "alpha", "beta", "gamma", "alpha"], dtype=object)
object_result = channels._factorize_categories(values)
unicode_result = channels._factorize_categories(values.astype("U5"))

assert object_result[0] == unicode_result[0]
np.testing.assert_array_equal(object_result[1], unicode_result[1])
np.testing.assert_array_equal(object_result[2], unicode_result[2])


def test_object_factorization_preserves_nul_distinct_categories():
"""NUL-containing strings retain Python equality semantics."""
categories, codes, counts = channels._factorize_categories(
np.array(["a", "a\x00", "a", "a\x00"], dtype=object)
)

assert categories == ["a", "a\x00"]
np.testing.assert_array_equal(codes, [0, 1, 0, 1])
assert counts is None


def test_near_unique_object_strings_do_not_run_full_width_scan(monkeypatch):
"""Near-unique object columns are rejected before full normalization."""
values = np.array([f"id-{i}" for i in range(5000)], dtype=object)
original = channels._object_string_width
calls: list[bool] = []

def record(arr, *, sample=False):
calls.append(sample)
return original(arr, sample=sample)

monkeypatch.setattr(channels, "_object_string_width", record)
channels._factorize_categories(values)

assert calls == [True]


@pytest.mark.parametrize(
"values",
[
np.array(["a", None, "a"], dtype=object),
np.array(["a", 1, "a"], dtype=object),
np.array([b"a", b"b", b"a"], dtype=object),
],
)
def test_mixed_object_categories_keep_the_fallback(values):
"""Values without exact string semantics remain on the safe fallback."""
categories, codes, counts = channels._factorize_categories(values)

assert len(categories) == len(set(categories))
assert len(codes) == len(values)
assert counts is None


def test_the_probe_still_reads_a_column_that_actually_looks_like_paint():
column = np.array(["#ff0000", "#00ff00", "#0000ff"])
rgba = channels._literal_color_rgba(column)
Expand Down