From b77b8e32a72d97309103d07ecbed9366029b913b Mon Sep 17 00:00:00 2001 From: Alek Petuskey Date: Wed, 22 Jul 2026 00:20:52 -0700 Subject: [PATCH] Skip inactive animation key encoding --- CHANGELOG.md | 10 ++ benchmarks/test_codspeed_animation.py | 30 +++- python/xy/_payload.py | 12 +- python/xy/_trace.py | 9 +- python/xy/components.py | 67 ++++--- spec/design/animation.md | 30 +++- tests/test_animation.py | 242 +++++++++++++++++++++++++- 7 files changed, 352 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da223dcb..4fb57689 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -142,6 +142,16 @@ in the README). contract without importing the widget stack. ### Changed +- **Inactive animation keys no longer add O(N) build work or 8 bytes/row to + payloads.** Mark keys are resolved, digested, retained, and shipped only for + an effective keyed update (a policy exists, is not explicitly disabled, + uses `match="key"`, and has updates enabled); `enabled="auto"` remains + eligible. Keyed marks above the 200k browser-match bound skip digesting and + reuse the existing explicit `snap:key-limit`/`snap:aggregate` fallback. + Active, bounded key validation and binary identity bytes are unchanged. On + a 100k-row disabled-animation scatter, median local chart construction fell + from 76.0 ms to 0.25 ms, payload assembly from 0.224 ms to 0.129 ms, and the + payload from 1.6 MB to 0.8 MB. - **Responsive, author-defeatable browser chrome.** XY's visual defaults now live in a low-priority cascade layer, so Tailwind utilities, ordinary author CSS, and slot styles override them without `!important`. Long legends remain diff --git a/benchmarks/test_codspeed_animation.py b/benchmarks/test_codspeed_animation.py index cc308687..6f4d40ad 100644 --- a/benchmarks/test_codspeed_animation.py +++ b/benchmarks/test_codspeed_animation.py @@ -2,7 +2,8 @@ Browser frame pacing and GPU allocation lifetime are measured by ``bench_animation.py``. These rows isolate the Python work: stable identity -encoding and the extra binary columns in an otherwise identical payload. +encoding, inactive-key chart construction, and the extra binary columns in an +otherwise identical payload. """ from __future__ import annotations @@ -28,11 +29,12 @@ def animation_data() -> tuple[np.ndarray, np.ndarray, list[str]]: def payload_figures(animation_data): x, y, keys = animation_data plain = xy.scatter_chart(xy.scatter(x=x, y=y)).figure() + inactive = xy.scatter_chart(xy.scatter(x=x, y=y, key=keys, animation=False)).figure() animated = xy.scatter_chart( xy.scatter(x=x, y=y, key=keys), xy.animation(match="key", duration=250), ).figure() - return plain, animated + return plain, inactive, animated def test_animation_encode_100k_stable_keys(benchmark, animation_data) -> None: @@ -43,15 +45,35 @@ def test_animation_encode_100k_stable_keys(benchmark, animation_data) -> None: def test_animation_plain_payload_100k(benchmark, payload_figures) -> None: - plain, _animated = payload_figures + plain, _inactive, _animated = payload_figures spec, blob = benchmark(plain.build_payload) assert spec["traces"][0]["n_marks"] == N assert "keys" not in spec["traces"][0] assert blob +def test_animation_inactive_key_build_100k(benchmark, animation_data) -> None: + x, y, keys = animation_data + + def build(): + return xy.scatter_chart(xy.scatter(x=x, y=y, key=keys, animation=False)).figure() + + figure = benchmark(build) + assert figure.traces[0].transition_keys is None + + +def test_animation_inactive_key_payload_100k(benchmark, payload_figures) -> None: + _plain, inactive, _animated = payload_figures + spec, blob = benchmark(inactive.build_payload) + trace = spec["traces"][0] + assert trace["n_marks"] == N + assert "keys" not in trace + assert all(column.get("dtype") != "u32" for column in spec["columns"]) + assert len(blob) == N * 8 + + def test_animation_keyed_payload_100k(benchmark, payload_figures) -> None: - _plain, animated = payload_figures + _plain, _inactive, animated = payload_figures spec, blob = benchmark(animated.build_payload) trace = spec["traces"][0] assert trace["n_marks"] == N diff --git a/python/xy/_payload.py b/python/xy/_payload.py index f6c10eaa..1337e852 100644 --- a/python/xy/_payload.py +++ b/python/xy/_payload.py @@ -302,9 +302,13 @@ def _transition_entry( if t.animation is not None and "animation" not in entry: entry["animation"] = dict(t.animation) keys = t.transition_keys if key_values is None else key_values - if keys is not None and entry.get("tier") != "direct": + has_key_metadata = keys is not None or t.transition_key_fallback is not None + if has_key_metadata and entry.get("tier") != "direct": entry["animation_fallback"] = "snap:aggregate" return entry + if t.transition_key_fallback is not None: + entry["animation_fallback"] = t.transition_key_fallback + return entry if keys is not None: values = keys if key_values is not None or sel is None else keys[sel] if len(values) == int(entry.get("n_marks", len(values))): @@ -425,7 +429,7 @@ def _emit_line( xv, yv = xv[finite], yv[finite] entry = self._base_entry(t, pw, xv, yv, tier, self._default_styled(t)) # Attach direct keys in the same finite/log-filtered row order. - if t.transition_keys is not None: + if t.transition_keys is not None or t.transition_key_fallback is not None: self._transition_entry(entry, t, pw, sel) return entry @@ -441,7 +445,7 @@ def _emit_area( if len(sel) != len(xv): xv, yv, bv = xv[sel], yv[sel], bv[sel] entry = self._base_entry(t, pw, xv, yv, tier, self._default_styled(t)) - if t.transition_keys is not None: + if t.transition_keys is not None or t.transition_key_fallback is not None: self._transition_entry(entry, t, pw, sel) entry["base"] = pw.ship(bv, t.base) return entry @@ -469,7 +473,7 @@ def _emit_scatter( sel = np.flatnonzero(visible) if sel is None else sel[visible] xv, yv = xv[visible], yv[visible] entry = self._base_entry(t, pw, xv, yv, "direct", dict(t.style)) - if t.transition_keys is not None: + if t.transition_keys is not None or t.transition_key_fallback is not None: self._transition_entry(entry, t, pw, sel) entry["color"], entry["size"] = self._ship_channels(t, sel, pw.ship_scalar, pw.ship_u8) self._ship_trace_styles(entry, t, sel, pw) diff --git a/python/xy/_trace.py b/python/xy/_trace.py index 14e4e4d5..f2c1354f 100644 --- a/python/xy/_trace.py +++ b/python/xy/_trace.py @@ -45,11 +45,14 @@ class Trace: # edgecolors="face" and is resolved against color_ch by the renderers. stroke_ch: Optional[ColorChannel] = None size_ch: Optional[SizeChannel] = None # scatter size encoding - # Declarative data-transition metadata. Keys are two uint32 words per - # canonical row (a deterministic 64-bit digest), kept out of the f64 - # column store because they are identity rather than numeric geometry. + # Declarative data-transition metadata. Active, bounded keys are two + # uint32 words per canonical row (a deterministic 64-bit digest), kept out + # of the f64 column store because they are identity rather than numeric + # geometry. An over-limit mark skips that digest and carries the existing + # explicit wire fallback separately. animation: Optional[dict[str, Any]] = None transition_keys: Optional[Any] = None + transition_key_fallback: Optional[str] = None # Direct, final-unit instance attributes (alpha override, opacity, widths, # symbols, corner radii). Constants stay in ``style`` and cost no buffer. style_channels: dict[str, StyleChannel] = field(default_factory=dict) diff --git a/python/xy/components.py b/python/xy/components.py index 6d882986..8b9ed840 100644 --- a/python/xy/components.py +++ b/python/xy/components.py @@ -46,6 +46,7 @@ from . import _validate, channels, export, styles from ._figure import Figure, Selection from ._typing import ArrayLike, ColorLike, Scalar, TableLike +from .config import MAX_ANIMATION_MATCH_ROWS from .dom import CHART_DOM_SLOTS, validate_dom_slots # Shared validators (single source of truth in `_validate`); these aliases keep @@ -3697,38 +3698,48 @@ def _apply_mark_transition_metadata( else: raise ValueError(f"{mark.kind} animation must be xy.animation(...), bool, or None") effective = {**(chart_spec or {}), **(mark_spec or {})} - if ( - effective.get("enabled") is not False + can_key_match = ( + (chart_spec is not None or mark_spec is not None) + and effective.get("enabled") is not False and effective.get("match") == "key" - and mark.key is None - ): + and effective.get("update") != "none" + ) + if can_key_match and mark.key is None: raise ValueError(f"{mark.kind} animation match='key' requires key=") - keys: np.ndarray | None = None - if mark.key is not None: - raw = ( - _resolve(data, mark.key, context=f"{mark.kind}.key") - if isinstance(mark.key, str) - else mark.key - ) - if not traces: - raise ValueError( - f"{mark.kind} key cannot be attached because the mark emitted no traces" - ) - expected = int(traces[0].n_points) - keys = _encode_transition_keys(raw, expected, f"{mark.kind} key") - if 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: - 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 + # A declarative key is inert unless this mark can actually use keyed + # update matching. In particular, do not resolve a column name or run the + # per-row canonicalization/digest loop for first-paint-only keys. + if not can_key_match: + return + raw = ( + _resolve(data, mark.key, context=f"{mark.kind}.key") + if isinstance(mark.key, str) + else mark.key + ) + if not traces: + raise ValueError(f"{mark.kind} key cannot be attached because the mark emitted no traces") + expected = int(traces[0].n_points) + if expected > MAX_ANIMATION_MATCH_ROWS: + # The browser will snap rather than allocate an unbounded identity + # map. Record that existing wire fallback without paying to digest + # keys which cannot be shipped or consumed. + for trace in traces: + trace.transition_key_fallback = "snap:key-limit" + return + keys = _encode_transition_keys(raw, expected, f"{mark.kind} key") + if 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: + 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 def _colorbar_source_title(mark: Mark) -> Optional[str]: diff --git a/spec/design/animation.md b/spec/design/animation.md index 53e95138..9e6a503e 100644 --- a/spec/design/animation.md +++ b/spec/design/animation.md @@ -58,15 +58,27 @@ points are dropped. Layouts that cannot share positional buffers record a `key=` is accepted by line, area, bar, column, scatter, error-band, and errorbar marks. It may be an array or a column name resolved through `data=`. -`match="key"` requires a key on every effectively keyed mark. +`match="key"` requires a key on every effectively keyed update. A key is +effective only when a chart- or mark-level animation policy is present, its +merged `enabled` value is not explicitly `False`, its merged `match` is +`"key"`, and `update` is not `"none"`. `enabled="auto"` remains effective +because reduced-motion preference is resolved only in the browser. Supplied +keys outside that effective policy are inert: Python does not resolve a key +column, validate/digest its rows, retain it on the trace, or ship identity +columns. This includes first paint with no animation policy, index/append +matching, disabled chart or mark policies, and update-disabled policies. Keys are canonicalized type-sensitively and hashed once in Python to a stable 64-bit identity, shipped as two binary `u32` columns. Strings, finite numbers, booleans, bytes, dates, datetimes, and NumPy equivalents are supported. -Missing, unsupported, wrong-length, or duplicate values fail during figure -construction. Line-like keys follow the same stable geometry sort as their -coordinates. Errorbar point keys are role-qualified after expansion so the -main segment and caps remain unique and stable. +For effective keyed updates within the matching limit, missing, unsupported, +wrong-length, or duplicate values fail during figure construction. Line-like +keys follow the same stable geometry sort as their coordinates. Errorbar point +keys are role-qualified after expansion so the main segment and caps remain +unique and stable. Above `MAX_ANIMATION_MATCH_ROWS`, a column-name key is still +resolved, but per-row canonicalization, validation, and digesting are skipped: +the identity cannot be consumed by the bounded browser matcher, so the trace +records `snap:key-limit` instead of paying unbounded Python-object work. The browser builds a bounded key→old-index map. `append` instead matches the old/new x identity and `index` pairs equal positions. Above @@ -152,13 +164,15 @@ an exact deterministic progress without starting a frame loop. ## 7. Verification and performance gates - `tests/test_animation.py` owns validation, serialization, identity, wire, - sorting, errorbar expansion, and deterministic-export contracts. + sorting, errorbar expansion, effective-policy overrides, bounded no-digest + fallbacks, and deterministic-export contracts. - `scripts/animation_smoke.py` exercises pixel-checked, ghost-free keyed interpolation, explicit partial-match fallback, GPU scratch buffers, rapid replacement, bounded lifetime, lifecycle balance (including destroy), and reduced motion in headless Chrome. -- `benchmarks/test_codspeed_animation.py` attributes key encoding and animated - payload build overhead separately from the plain payload path. +- `benchmarks/test_codspeed_animation.py` attributes inactive-key construction, + key encoding, and animated payload build overhead separately from the plain + payload path. - Browser frame/allocation measurements belong to the real-Chrome benchmark lane, not CodSpeed simulation; the animation smoke asserts the hard previous+next allocation bound. diff --git a/tests/test_animation.py b/tests/test_animation.py index 1bbd2355..fc26115a 100644 --- a/tests/test_animation.py +++ b/tests/test_animation.py @@ -154,6 +154,132 @@ def test_keyed_scatter_ships_identity_as_binary_u32_words() -> None: assert [column.get("dtype") for column in spec["columns"]].count("u32") == 2 +@pytest.mark.parametrize( + ("chart_animation", "mark_animation", "expected_keys"), + [ + pytest.param(None, None, False, id="no-policy"), + pytest.param(None, True, False, id="mark-enabled-without-key-match"), + pytest.param(xy.animation(match="index"), None, False, id="chart-index-match"), + pytest.param( + xy.animation(enabled=False, match="key"), + None, + False, + id="chart-disabled", + ), + pytest.param( + xy.animation(match="key", update="none"), + None, + False, + id="chart-update-none", + ), + pytest.param(xy.animation(match="key"), False, False, id="mark-disabled"), + pytest.param( + xy.animation(match="key"), + xy.animation(match="index"), + False, + id="mark-index-override", + ), + pytest.param( + xy.animation(match="key"), + xy.animation(match="key", update="none"), + False, + id="mark-update-none-override", + ), + pytest.param(None, xy.animation(match="key"), True, id="mark-key-match"), + pytest.param( + xy.animation(enabled="auto", match="key"), + None, + True, + id="chart-auto-key-match", + ), + pytest.param( + xy.animation(enabled=True, match="key"), + None, + True, + id="chart-enabled-key-match", + ), + pytest.param(xy.animation(match="key"), True, True, id="mark-enabled-inherits"), + pytest.param( + xy.animation(match="index"), + xy.animation(match="key"), + True, + id="mark-key-override", + ), + ], +) +def test_only_effective_keyed_updates_attach_transition_keys( + chart_animation: object | None, + mark_animation: object | None, + expected_keys: bool, +) -> None: + children = [ + xy.scatter( + x=[1.0, 2.0], + y=[3.0, 4.0], + key=["a", "b"], + animation=mark_animation, + ) + ] + if chart_animation is not None: + children.append(chart_animation) + figure = xy.scatter_chart(*children).figure() + + assert (figure.traces[0].transition_keys is not None) is expected_keys + spec, _ = figure.build_payload() + trace = spec["traces"][0] + assert ("keys" in trace) is expected_keys + assert sum(column.get("dtype") == "u32" for column in spec["columns"]) == ( + 2 if expected_keys else 0 + ) + + +@pytest.mark.parametrize( + ("chart_animation", "mark_animation"), + [ + pytest.param(None, None, id="no-policy"), + pytest.param(xy.animation(match="index"), None, id="index-match"), + pytest.param(xy.animation(enabled=False, match="key"), None, id="disabled"), + pytest.param(xy.animation(match="key", update="none"), None, id="no-update"), + pytest.param(xy.animation(match="key"), False, id="disabled-override"), + ], +) +def test_inactive_key_policies_do_not_digest( + monkeypatch, + chart_animation: object | None, + mark_animation: object | None, +) -> None: + def unexpected_digest(*_args, **_kwargs): + raise AssertionError("inactive animation key was digested") + + monkeypatch.setattr("xy.components._encode_transition_keys", unexpected_digest) + children = [ + xy.scatter( + x=[1.0, 2.0], + y=[3.0, 4.0], + key=["duplicate", "duplicate"], + animation=mark_animation, + ) + ] + if chart_animation is not None: + children.append(chart_animation) + + figure = xy.scatter_chart(*children).figure() + assert figure.traces[0].transition_keys is None + + +def test_inactive_key_column_is_not_resolved_or_validated() -> None: + figure = xy.scatter_chart( + xy.scatter( + x="x", + y="y", + key="missing-key-column", + data={"x": [1.0, 2.0], "y": [3.0, 4.0]}, + ) + ).figure() + + assert figure.traces[0].transition_keys is None + + def test_stable_keys_are_type_sensitive_and_deterministic() -> None: chart = xy.scatter_chart( xy.scatter(x=[1.0, 2.0, 3.0], y=[3.0, 4.0, 5.0], key=[1, 1.0, True]), @@ -183,7 +309,7 @@ def test_aggregate_tier_records_key_matching_fallback() -> None: y=[3.0, 4.0, 5.0], key=["a", "b", "c"], density=True, - animation=xy.animation(duration=90), + animation=xy.animation(duration=90, match="key"), ), xy.animation(match="key"), ) @@ -197,6 +323,111 @@ def test_aggregate_tier_records_key_matching_fallback() -> None: assert "keys" not in trace +@pytest.mark.parametrize( + ("mark", "expected_fallback"), + [ + pytest.param( + lambda: xy.scatter( + x=[1.0, 2.0, 3.0], + y=[3.0, 4.0, 5.0], + key=["a", "b", "c"], + density=False, + ), + "snap:key-limit", + id="scatter-direct", + ), + pytest.param( + lambda: xy.scatter( + x=[1.0, 2.0, 3.0], + y=[3.0, 4.0, 5.0], + key=["a", "b", "c"], + density=True, + ), + "snap:aggregate", + id="scatter-aggregate", + ), + pytest.param( + lambda: xy.line([1.0, 2.0, 3.0], [3.0, 4.0, 5.0], key=["a", "b", "c"]), + "snap:key-limit", + id="line", + ), + pytest.param( + lambda: xy.area([1.0, 2.0, 3.0], [3.0, 4.0, 5.0], key=["a", "b", "c"]), + "snap:key-limit", + id="area", + ), + pytest.param( + lambda: xy.error_band( + [1.0, 2.0, 3.0], + [2.0, 3.0, 4.0], + [4.0, 5.0, 6.0], + key=["a", "b", "c"], + ), + "snap:key-limit", + id="error-band", + ), + pytest.param( + lambda: xy.bar([1.0, 2.0, 3.0], [3.0, 4.0, 5.0], key=["a", "b", "c"]), + "snap:key-limit", + id="bar", + ), + pytest.param( + lambda: xy.column([1.0, 2.0, 3.0], [3.0, 4.0, 5.0], key=["a", "b", "c"]), + "snap:key-limit", + id="column", + ), + pytest.param( + lambda: xy.errorbar( + [1.0, 2.0, 3.0], + [3.0, 4.0, 5.0], + yerr=[0.1, 0.2, 0.3], + key=["a", "b", "c"], + ), + "snap:key-limit", + id="errorbar", + ), + ], +) +def test_over_limit_keys_skip_digest_and_record_fallback( + monkeypatch, + mark, + expected_fallback: str, +) -> None: + monkeypatch.setattr("xy.components.MAX_ANIMATION_MATCH_ROWS", 2) + + def unexpected_digest(*_args, **_kwargs): + raise AssertionError("over-limit animation key was digested") + + monkeypatch.setattr("xy.components._encode_transition_keys", unexpected_digest) + figure = xy.chart( + mark(), + xy.animation(match="key"), + ).figure() + + assert all(trace.transition_keys is None for trace in figure.traces) + assert all(trace.transition_key_fallback == "snap:key-limit" for trace in figure.traces) + spec, _ = figure.build_payload() + assert all(trace["animation_fallback"] == expected_fallback for trace in spec["traces"]) + assert all("keys" not in trace for trace in spec["traces"]) + assert all(column.get("dtype") != "u32" for column in spec["columns"]) + + +def test_over_limit_active_key_column_is_still_resolved(monkeypatch) -> None: + monkeypatch.setattr("xy.components.MAX_ANIMATION_MATCH_ROWS", 2) + chart = xy.scatter_chart( + xy.scatter( + x="x", + y="y", + key="missing-key-column", + data={"x": [1.0, 2.0, 3.0], "y": [3.0, 4.0, 5.0]}, + ), + xy.animation(match="key"), + ) + + with pytest.raises(ValueError, match=r"scatter\.key column 'missing-key-column'"): + chart.figure() + + def test_mark_animation_overrides_chart_defaults() -> None: chart = xy.chart( xy.scatter( @@ -279,6 +510,15 @@ def test_key_matching_requires_a_key_on_every_animated_mark() -> None: chart.figure() +def test_key_matching_without_updates_does_not_require_a_key() -> None: + figure = xy.line_chart( + xy.line(x=[1.0, 2.0], y=[3.0, 4.0]), + xy.animation(match="key", update="none"), + ).figure() + + assert figure.traces[0].transition_keys is None + + def test_line_keys_follow_the_geometry_sort_order() -> None: chart = xy.line_chart( xy.line(x=[3.0, 1.0, 2.0], y=[30.0, 10.0, 20.0], key=["c", "a", "b"]),