Skip to content
Merged
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
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ in the README).
that don't use them are byte-identical.

### Changed
- Stable animation `key=` identity planes are now retained and shipped only
when the resolved animation spec can actually key-match. `match` defaults to
`"index"`, so `key=` combined with a bare `xy.animation(...)`, with
`enabled=False`, or with no animation at all previously put two dead `u32`
columns in the payload — 8 B/row held for the widget lifetime and 8 B/row on
the wire (400 KB at 50k rows) that no client code read. Encoding still runs
in every case: duplicate-key and row-count errors are construction contract,
not animation policy, and are unchanged. Payloads that do key-match are
byte-identical.
- Stable animation `key=` identity encoding now uses one native Rust row scan
for homogeneous fixed-width strings, bytes, booleans, and signed or unsigned
integers, plus finite floating arrays, including non-native-endian NumPy
Expand Down
13 changes: 11 additions & 2 deletions python/xy/components.py
Original file line number Diff line number Diff line change
Expand Up @@ -4069,6 +4069,12 @@ def _apply_mark_transition_metadata(
):
raise ValueError(f"{mark.kind} animation match='key' requires key=")
keys: np.ndarray | None = None
# `match` defaults to "index", so a bare `xy.animation(...)` — or no
# animation at all — never key-matches. Encoding still runs for its
# uniqueness and typing contract, but the identity planes are dead weight
# nothing reads: 8 B/row retained for the widget lifetime and 8 B/row on
# the wire. Only carry them when the client can actually match on them.
key_matching = effective.get("enabled") is not False and effective.get("match") == "key"
if mark.key is not None:
raw = (
_resolve(data, mark.key, context=f"{mark.kind}.key")
Expand All @@ -4081,19 +4087,22 @@ def _apply_mark_transition_metadata(
)
expected = int(traces[0].n_points)
keys = _encode_transition_keys(raw, expected, f"{mark.kind} key")
if mark.kind in {"line", "area", "error_band"}:
if key_matching and mark.kind in {"line", "area", "error_band"}:
positions = _original_mark_positions(fig, mark, data, expected)
if positions is not None:
keys = keys[np.argsort(positions, kind="stable")]
for trace in traces:
trace.animation = None if mark_spec is None else dict(mark_spec)
if keys is not None:
# The per-trace row check is contract too, so it runs whether or
# not the planes are kept.
if trace.n_points != len(keys):
raise ValueError(
f"{mark.kind} key has {len(keys)} rows but emitted trace {trace.id} "
f"has {trace.n_points} logical rows"
)
trace.transition_keys = keys
if key_matching:
trace.transition_keys = keys


def _continuous_color_label(mark: Mark) -> Optional[str]:
Expand Down
10 changes: 9 additions & 1 deletion spec/design/animation.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,18 @@ The kernel separates "declined this data, use the oracle" from "this layout is
not in the ABI": only the first falls back, so a contract drift between the
ctypes gate and the Rust layout check raises instead of silently costing the
whole speedup.
Encoding runs whenever `key=` is given, because uniqueness and typing are
construction contract rather than animation policy. The resulting identity
planes are only retained and shipped when the *resolved* spec can key-match —
`match` defaults to `"index"`, so a bare `xy.animation(...)`, an
`enabled=False` chart, or a `key=` with no animation at all would otherwise
carry two u32 columns nothing reads, both in the widget's retained payload and
on the wire. Duplicate and row-count errors are unaffected by that skip.
Line-like keys follow the same stable geometry sort as their coordinates; the
encoder hands back Fortran-order planes that ship without a per-column copy,
but that sort and any finite-row selection reorder through NumPy advanced
indexing, which returns C-order and restores the copy at ship time.
indexing, which returns C-order and restores the copy at ship time. The sort is
skipped along with the planes when nothing will match on them.
Errorbar point keys are role-qualified after expansion so the main segment and
caps remain unique and stable.

Expand Down
94 changes: 93 additions & 1 deletion tests/test_animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -460,14 +460,106 @@ def test_native_transition_key_argument_errors_are_loud() -> None:
_native._lib.xy_transition_keys_fixed = original


def _keyed_scatter(animation, n: int = 8):
x = [float(i) for i in range(n)]
marks = [xy.scatter(x=x, y=x, key=[f"k{i}" for i in range(n)])]
return xy.scatter_chart(*marks, *([animation] if animation is not None else [])).figure()


@pytest.mark.parametrize(
("animation", "ships"),
[
pytest.param(xy.animation(match="key"), True, id="match-key"),
pytest.param(xy.animation(match="index"), False, id="match-index"),
# `match` defaults to "index", so a bare animation() never key-matches.
pytest.param(xy.animation(duration=250), False, id="match-defaulted"),
pytest.param(xy.animation(match="key", enabled=False), False, id="disabled"),
pytest.param(None, False, id="no-animation-spec"),
],
)
def test_identity_planes_ship_only_when_the_client_can_key_match(animation, ships) -> None:
"""`key=` alone must not put two dead u32 columns on the wire."""
figure = _keyed_scatter(animation)
spec, _buffers = figure.build_payload_split()
trace = spec["traces"][0]

assert ("keys" in trace) is ships
assert (figure.traces[0].transition_keys is not None) is ships


@pytest.mark.parametrize(
"animation",
[
pytest.param(xy.animation(match="index"), id="match-index"),
pytest.param(xy.animation(enabled=False), id="disabled"),
pytest.param(None, id="no-animation-spec"),
],
)
def test_key_validation_still_runs_when_planes_are_skipped(animation) -> None:
"""Uniqueness and typing are construction contract, not animation policy."""
marks = xy.scatter(x=[1.0, 2.0, 3.0], y=[1.0, 2.0, 3.0], key=["a", "b", "a"])
with pytest.raises(ValueError, match="duplicate value at rows 0 and 2"):
xy.scatter_chart(marks, *([animation] if animation is not None else [])).figure()

bad = xy.scatter(x=[1.0, 2.0], y=[1.0, 2.0], key=[object(), object()])
with pytest.raises(ValueError, match="animation key values must be"):
xy.scatter_chart(bad, *([animation] if animation is not None else [])).figure()


def test_key_matching_payload_is_unchanged_by_the_skip() -> None:
"""The path that does key-match must be byte-identical to before."""
spec, blob = _keyed_scatter(xy.animation(match="key"), n=32).build_payload()
trace = spec["traces"][0]
lo = _column(blob, spec, trace["keys"]["lo"])
hi = _column(blob, spec, trace["keys"]["hi"])
expected = _python_transition_key_reference([f"k{i}" for i in range(32)])
np.testing.assert_array_equal(lo, expected[:, 0])
np.testing.assert_array_equal(hi, expected[:, 1])


def test_mark_animation_spec_clobbers_chart_level_fields() -> None:
"""KNOWN BUG (reflex-dev/xy#329) — pinned so a fix is a deliberate change.

`Animation.to_spec()` emits every field, and both the Python merge and the
client's `{...spec.animation, ...trace.animation}` are plain dict spreads.
So a mark-level `xy.animation(duration=90)` resets `match`, `easing`,
`enter`, `update`, and `interpolate` to their defaults, silently turning
off the chart-level `match="key"` the caller asked for.
"""
figure = xy.scatter_chart(
xy.scatter(
x=[1.0, 2.0],
y=[1.0, 2.0],
key=["a", "b"],
animation=xy.animation(duration=90),
),
xy.animation(match="key", easing="linear"),
).figure()
spec, _ = figure.build_payload_split()
resolved = {**spec.get("animation", {}), **(spec["traces"][0].get("animation") or {})}

assert resolved["match"] == "index" # caller asked for "key"
assert resolved["easing"] == "ease-out" # caller asked for "linear"
assert resolved["duration"] == 90.0 # the one field they meant to set

# The same clobbering suppresses the match='key' requires key= guard.
xy.scatter_chart(
xy.scatter(x=[1.0, 2.0], y=[1.0, 2.0], animation=xy.animation(duration=90)),
xy.animation(match="key"),
).figure()


def test_aggregate_tier_records_key_matching_fallback() -> None:
chart = xy.scatter_chart(
xy.scatter(
x=[1.0, 2.0, 3.0],
y=[3.0, 4.0, 5.0],
key=["a", "b", "c"],
density=True,
animation=xy.animation(duration=90),
# `match="key"` has to be restated here: a mark-level animation
# spec is a full dict, so it resets every field the chart set.
# See test_mark_animation_spec_clobbers_chart_level_fields.
animation=xy.animation(duration=90, match="key"),
),
xy.animation(match="key"),
)
Expand Down
Loading