Make customization survive the trip to a file - #315
Merged
Conversation
An audit of xy's customizability across eleven dimensions found one theme
behind most of it: options the WebGL client honors, the static exporters
silently drop. A user styles a chart in a notebook, exports it for a report,
and gets something different — with no warning, and no way to notice until
someone reads the report.
Fixed, each verified against the live client:
- The categorical palette cycled per *trace*, not per series. A box is four
traces and a stem is two, so four box series under a four-color
xy.theme(palette=...) all wore palette[0], and eight box series drew two
colors out of the built-in eight. Adding an outlier to one series repainted
a different one. Marks now take one slot per logical series, and an explicit
color= takes none, matching matplotlib's property cycle.
- x_axis/y_axis format= reached the browser and nothing else: "$,.0f" read
$1,000,000 on screen and 1.0e6 in the PNG. The exporters now share the
client's grammar, and the left gutter grows for labels that need the room.
- A categorical color= channel produced no legend in any export — one row
bearing the trace name and one color, beside marks painted in three.
- text= on hline/vline/bands/threshold/arrow was dropped by the exporters, and
xy.marker() drew nothing at all. Placement is now one helper shared by both,
ported from the client's own label pass.
- Log-axis decades below 1.0 all rendered "0": precision came from the tick
step, which means nothing on a multiplicative axis.
- color=["#ff0000", "#00ff00"] was factorized into categories, sorted
alphabetically, and repainted from the palette. Written-out colors now paint
themselves; named colors stay categorical, since a column of "red"/"green"
is ordinary data.
Added: xy.theme(palette={category: color}) pins colors to labels rather than
positions, so a category keeps its color across marks and across facet panels
where a positional cycle silently recolors whatever is left.
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR adds category-mapped palettes and per-series color allocation, recognizes literal CSS color lists, updates static SVG/PNG formatting, legends, annotations, and layout, and expands native glyph coverage with visible U+FFFD fallback rendering. ChangesPalette and color resolution
Static export parity
Glyph coverage expansion
Estimated code review effort: 4 (Complex) | ~50 minutes Sequence Diagram(s)sequenceDiagram
participant Theme
participant Figure
participant Marks
participant ColorResolver
participant SVGExporter
Theme->>Figure: configure positional or category-mapped palette
Figure->>Figure: advance per-series palette cursor
Figure-->>Marks: return series color
Marks->>ColorResolver: resolve series or categorical color
ColorResolver-->>Marks: return resolved channel
Marks->>SVGExporter: emit traces and annotations
SVGExporter->>SVGExporter: format axes and expand legend rows
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
python/xy/_raster.py (1)
1089-1091: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
if text_phase: passas a skip guard reads as dead code.Inverting the guard makes the two-phase split explicit.
♻️ Alternative shape
- if text_phase: - pass - elif ann.get("kind") == "rule": + kind = ann.get("kind") + if not text_phase and kind == "rule":(and switch the remaining
elifs toelif not text_phase and kind == ..., or wrap the geometry chain in a singleif not text_phase:block.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/xy/_raster.py` around lines 1089 - 1091, Update the conditional chain around the text-phase handling so geometry processing is explicitly gated by not text_phase, replacing the empty if text_phase/pass branch. Preserve the existing rule and subsequent annotation-kind handling, using either an if not text_phase wrapper or corresponding elif not text_phase conditions.tests/test_svg_export.py (1)
803-814: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe first
<rect x=...>is an assumption, not the plot rect.
re.searchgrabs whichever rect the emitter happens to write first (currently the clipPath rect). If background rects ever gain anx, this silently measures the wrong thing — and.group(1)onNonegives anAttributeErrorrather than a readable failure. Asserting against the clipPath rect explicitly (or calling_svg.layout(spec)) would pin the intent.startsis also only used for its minimum.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_svg_export.py` around lines 803 - 814, The test should identify the actual plot rectangle rather than assuming the first emitted <rect> is the plot rect. Update test_formatted_labels_widen_the_gutter_instead_of_running_off_the_canvas to explicitly target the clipPath/plot rectangle or use _svg.layout(spec), and make missing matches fail with a clear assertion; simplify starts to compute only the minimum value needed by the assertion.tests/test_png_export.py (1)
934-950: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer the
monkeypatchfixture over hand-rolled attribute swaps.
monkeypatch.setattr(raster._Cmd, "text", record_text)restores automatically, drops thetry/finallyin both tests, and avoids leaking a patched class attribute if the restore is ever skipped. Alsoargs[-1]silently records the wrong value if a caller ever passess=by keyword — capturing the named parameter is sturdier.Also applies to: 967-978
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_png_export.py` around lines 934 - 950, The tests’ manual raster command patching is fragile and records text arguments incorrectly when passed by keyword. In the affected tests around chart.to_png, replace hand-written attribute swaps and try/finally restoration with the pytest monkeypatch fixture and monkeypatch.setattr for raster._Cmd.text and raster._Cmd.point; update record_text to capture the named s value explicitly while preserving the existing call recording and delegation behavior.python/xy/_figure.py (1)
183-194: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate dict-or-list palette flattening logic across two files.
Both sites independently implement "flatten a
{category: color}or list palette into a positional color cycle," differing only in what happens when unset. Extracting one shared helper (e.g. inconfig.py) would prevent the two from drifting if the flattening rule changes again.
python/xy/_figure.py#L183-L194:Figure.palette_cycle— the "returnNone-if-unset" variant; a good candidate to become the canonical implementation thatchannels.resolve_colordelegates to (passing its own default).python/xy/channels.py#L536-L544:resolve_color's localcyclederivation — replace with a call into the shared helper, falling back toconfig.DEFAULT_PALETTEwhen the result is empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/xy/_figure.py` around lines 183 - 194, Extract the dict-or-list palette flattening from Figure.palette_cycle into a shared helper, preserving its None-when-unset behavior; update python/xy/_figure.py lines 183-194 to use or expose that canonical implementation. Replace the local cycle derivation in python/xy/channels.py lines 536-544 within resolve_color with the shared helper, falling back to config.DEFAULT_PALETTE when the helper returns None or an empty result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@js/src/30_ticks.ts`:
- Around line 225-229: Update the sub-unit log fallback condition in the axis
tick formatting logic to evaluate the numeric core of formatted, rather than
calling Number on the affixed string. Reuse the parsed core if available, or
strip prefixes and suffixes before checking for zero, so values such as "$0" and
"0K" trigger fmtLog(v) while preserving the existing null and nonzero behavior.
In `@python/xy/_svg.py`:
- Around line 590-595: Update the sub-unit log fallback checks in
python/xy/_svg.py lines 590-595 and js/src/30_ticks.ts lines 225-229 to extract
and compare the numeric core of the formatted label, excluding prefixes and
suffixes such as "$" and "K"; preserve the existing zero-collapse behavior and
route both runtimes to _fmt_log/fmtLog consistently.
- Around line 2166-2171: The SVG marker exporter’s stroke fallback differs from
the documented white marker outline. In python/xy/_svg.py lines 2166-2171,
update the stroke_color fallback in the marker stroke_attr construction to use
"`#ffffff`" instead of the current color value; python/xy/_raster.py lines
1150-1167 requires no direct change because the SVG correction aligns both
exporters.
In `@python/xy/channels.py`:
- Around line 620-658: Update the palette fallback in the palette_map branch so
unmapped categories never reuse colors already pinned by the map when spare
default colors are exhausted. Build the fallback cycle from default colors
excluding spent values, and only reuse colors when no non-spent colors remain;
preserve the existing behavior for available spare colors and the warning for
unmapped categories.
---
Nitpick comments:
In `@python/xy/_figure.py`:
- Around line 183-194: Extract the dict-or-list palette flattening from
Figure.palette_cycle into a shared helper, preserving its None-when-unset
behavior; update python/xy/_figure.py lines 183-194 to use or expose that
canonical implementation. Replace the local cycle derivation in
python/xy/channels.py lines 536-544 within resolve_color with the shared helper,
falling back to config.DEFAULT_PALETTE when the helper returns None or an empty
result.
In `@python/xy/_raster.py`:
- Around line 1089-1091: Update the conditional chain around the text-phase
handling so geometry processing is explicitly gated by not text_phase, replacing
the empty if text_phase/pass branch. Preserve the existing rule and subsequent
annotation-kind handling, using either an if not text_phase wrapper or
corresponding elif not text_phase conditions.
In `@tests/test_png_export.py`:
- Around line 934-950: The tests’ manual raster command patching is fragile and
records text arguments incorrectly when passed by keyword. In the affected tests
around chart.to_png, replace hand-written attribute swaps and try/finally
restoration with the pytest monkeypatch fixture and monkeypatch.setattr for
raster._Cmd.text and raster._Cmd.point; update record_text to capture the named
s value explicitly while preserving the existing call recording and delegation
behavior.
In `@tests/test_svg_export.py`:
- Around line 803-814: The test should identify the actual plot rectangle rather
than assuming the first emitted <rect> is the plot rect. Update
test_formatted_labels_widen_the_gutter_instead_of_running_off_the_canvas to
explicitly target the clipPath/plot rectangle or use _svg.layout(spec), and make
missing matches fail with a clear assertion; simplify starts to compute only the
minimum value needed by the assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 73794e84-6f0b-403b-84d9-a4e2bd255ce1
⛔ Files ignored due to path filters (1)
spec/assets/export-parity-before-after.pngis excluded by!**/*.png
📒 Files selected for processing (15)
CHANGELOG.mddocs/styling/customize.mdjs/src/30_ticks.tspython/xy/_figure.pypython/xy/_hosts.pypython/xy/_payload.pypython/xy/_raster.pypython/xy/_svg.pypython/xy/channels.pypython/xy/components.pypython/xy/marks.pyspec/api/styling.mdtests/test_custom_ramps_and_palette.pytests/test_png_export.pytests/test_svg_export.py
CodSpeed caught a 68.8% regression on `test_first_payload_scatter_categorical_color`. The cause is in this branch: the probe that decides whether `color=` is a list of CSS colors ran `arr.tolist()` unconditionally, ahead of `_factorize_categories` — which goes to real trouble to identify equal records in Rust precisely so it never creates N Python objects. So every categorical scatter paid for a full materialization it did not need. Measured at ~39% of the payload build for a 500k-row column. A category column fails the test on its very first value, so gate on entry zero: `["setosa", ...]` now costs O(1) and only an array that already looks like paint pays for the full scan. Requiring entry zero to match is exactly as strict as the `all(...)` that follows, which already demands every entry be a color string, so no input changes classification. Local timings at the benchmark's 100k rows, median of 12: origin/main 2.00 ms before (probe alone was 7.9 ms at 500k) after 2.03 ms Pinned with a test rather than a timing assertion: an ndarray subclass whose `tolist` raises.
# Conflicts: # CHANGELOG.md
The native rasterizer bakes its glyphs into src/font.rs because the Rust core avoids FreeType. A character outside that atlas got no glyph AND no advance — so it did not render as a box, it vanished. `Zürich` exported as `Zrich`, a `format="€,.0f"` axis lost its symbol on every tick, and a CJK label exported as blank space, all while the same figure's SVG rendered them correctly. Two changes: - Extend the atlas to the Latin-1 Supplement and Latin Extended-A letters — every language written in the Latin script — plus non-ASCII currency. The generator takes ranges rather than a hand-typed string so the coverage is auditable. 205 -> 424 glyphs, ~48 KB of baked coverage, +4% on the core dylib. - Substitute U+FFFD for anything still unbaked (CJK, Cyrillic, emoji) instead of dropping it. §28 asks that a decision the engine makes on the user's behalf be visible, and a replacement box is the visible form of "this renderer cannot draw that". Zero-width and control characters stay dropped — they have nothing to show. Currency is not incidental: `y_axis(format="€,.0f")` shipped two commits ago, and its symbol was being deleted from every tick of the exported PNG. spec/api/styling.md documented the silent-skip behavior precisely, so it is updated rather than left contradicting the code.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
python/xy/channels.py (1)
595-602: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep the literal-color fast path on the required wire encoding.
This path now routes CSS arrays into
direct_rgba, whose shipped spec usesdtype: "u8". That produces an RGBA8 buffer instead of the required raw f32 transport.As per coding guidelines,
**/*.{rs,py,ts}must transfer data as raw f32 buffers while retaining f64 canonical CPU values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/xy/channels.py` around lines 595 - 602, The literal CSS-color fast path in _literal_color_rgba and the surrounding channel construction must preserve raw f32 transport while retaining f64 canonical CPU values. Update the direct_rgba representation or its encoding metadata so this path emits the required raw f32 buffer instead of the shipped u8 RGBA8 wire format, without changing per-point color order or bypassing the fast path.Source: Coding guidelines
python/xy/marks.py (1)
665-669: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSnapshot the series cursor before allocating a default color.
next_series_color()is stateful, but these direct calls occur before the mark transaction is checkpointed. For example,line()consumes a color at Line 826 and checkpoints only at Line 831; a later validation or ingest failure can therefore shift the next successful trace to the wrong palette slot.Checkpoint before allocation, or make rollback restore the cursor from a pre-allocation snapshot.
Also applies to: 825-826, 902-903, 987-988, 1088-1088, 1327-1327, 1629-1630, 1755-1755, 1918-1919, 2113-2113, 2513-2513
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/xy/marks.py` around lines 665 - 669, Update the mark color-allocation paths, including the visible series_colors comprehension and the corresponding direct next_series_color() calls in the affected mark methods, to snapshot the series-color cursor before allocation and restore it whenever validation or ingest fails. Ensure failed mark transactions leave the cursor exactly as it was before default-color allocation, while successful traces retain their allocated colors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@python/xy/channels.py`:
- Around line 595-602: The literal CSS-color fast path in _literal_color_rgba
and the surrounding channel construction must preserve raw f32 transport while
retaining f64 canonical CPU values. Update the direct_rgba representation or its
encoding metadata so this path emits the required raw f32 buffer instead of the
shipped u8 RGBA8 wire format, without changing per-point color order or
bypassing the fast path.
In `@python/xy/marks.py`:
- Around line 665-669: Update the mark color-allocation paths, including the
visible series_colors comprehension and the corresponding direct
next_series_color() calls in the affected mark methods, to snapshot the
series-color cursor before allocation and restore it whenever validation or
ingest fails. Ensure failed mark transactions leave the cursor exactly as it was
before default-color allocation, while successful traces retain their allocated
colors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 690b3f96-7f0c-4904-a9c9-4950853d3cc5
📒 Files selected for processing (9)
CHANGELOG.mddocs/styling/customize.mdpython/xy/_raster.pypython/xy/_svg.pypython/xy/channels.pypython/xy/components.pypython/xy/marks.pyspec/api/styling.mdtests/test_custom_ramps_and_palette.py
🚧 Files skipped from review as they are similar to previous changes (6)
- spec/api/styling.md
- docs/styling/customize.md
- CHANGELOG.md
- tests/test_custom_ramps_and_palette.py
- python/xy/components.py
- python/xy/_raster.py
Merging this PR will not alter performance
Comparing 
Footnotes
|
Review catch, and a genuine crash. The sub-unit log fallback asked "did this
spec collapse the value to zero?" by comparing the FORMATTED label against
zero — but the grammar allows prefixes and suffixes, so a `format="$,.0f"`
axis renders a sub-unit decade as `"$0"`.
float("$0") raises ValueError and takes the whole render down
Number("$0") is NaN, so `=== 0` is false and the collapsed label ships
Two different wrong answers out of the one layer whose job is to keep the
browser and the exporters identical. `_collapsed_to_zero` / `collapsedToZero`
now strip everything that is not part of the number — affixes and group
separators alike — before the comparison, in both runtimes.
Also from review: a `{category: color}` palette that pins all eight built-in
colors leaves nothing distinct for an unmapped category, so it necessarily
reuses one that is already spoken for. That is unavoidable with eight
defaults, but it should not be silent — the unmapped-category warning now says
when the fallback repeats a pinned color.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/api/styling.md`:
- Around line 749-753: Update the Unicode coverage description in the
documentation paragraph to avoid claiming support for every language written in
the Latin script. Replace that wording with “many Latin-script languages” or
explicitly enumerate only the supported Unicode ranges, while preserving the
examples for currency formatting and Zürich tick labels.
In `@spec/design/rust-engine.md`:
- Around line 91-96: Update section §2.1 in the Rust engine specification to
match the expanded glyph contract: document U+FFFD fallback for unsupported
characters instead of silent deletion, and replace the outdated 205 glyph/110
extra-codepoint coverage figures with 424 glyph records and 329 extra
codepoints. Keep the section consistent with the generated font.rs module
summary.
In `@src/raster.rs`:
- Around line 1443-1461: The glyph_index function should map locale whitespace
separators, including NBSP and narrow NBSP, to the ASCII space glyph before
applying the replacement-character fallback. Preserve dropping zero-width and
control characters, and retain existing atlas lookup behavior for all other
characters.
In `@tests/test_png_export.py`:
- Around line 1012-1013: Update test_zero_width_characters_still_draw_nothing to
represent the U+200B zero-width space with an explicit Unicode escape in the
string literal, preserving the existing comparison with _title_png("X").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f679cdd-0fad-4942-a4ba-869e6c893b66
⛔ Files ignored due to path filters (1)
spec/assets/raster-glyph-coverage.pngis excluded by!**/*.png
📒 Files selected for processing (7)
CHANGELOG.mdscripts/gen_font.pyspec/api/styling.mdspec/design/rust-engine.mdsrc/font.rssrc/raster.rstests/test_png_export.py
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@js/src/30_ticks.ts`:
- Around line 228-233: The collapsedToZero logic in js/src/30_ticks.ts#L228-L233
incorrectly treats digit-bearing affixes as part of the numeric value; use the
parsed format prefix/suffix from fmtNumberSpec, or return the numeric portion
from it, before testing whether the formatted value is zero. Add a regression
case in tests/test_svg_export.py#L803-L821 using an affix such as v2=.0f and
assert that the sub-unit magnitude label is retained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b4eecd1a-0543-4065-a805-088a8686792c
📒 Files selected for processing (4)
js/src/30_ticks.tspython/xy/_svg.pypython/xy/channels.pytests/test_svg_export.py
🚧 Files skipped from review as they are similar to previous changes (2)
- python/xy/channels.py
- python/xy/_svg.py
Second review round, three real catches:
- Substituting U+FFFD for every unbaked codepoint boxed the non-ASCII SPACES
too. Locale-aware number formatting emits NBSP (U+00A0) and narrow NBSP
(U+202F) as group separators, so those arrive through ordinary tick labels —
and whitespace is drawn by its advance, not its shape, so a box is a worse
answer than the space it stands for. Whitespace now maps to the ASCII space
glyph; control and zero-width characters stay dropped; everything else still
boxes.
- "Every language written in the Latin script" was an over-claim. Latin-1
Supplement plus Latin Extended-A carries Western, Central and Northern
European orthographies, but Vietnamese and Romanian's ș/ț live in later
blocks and still box. Both spec statements now say which blocks, and which
languages that leaves out.
- rust-engine.md §2.1 still documented 205 glyphs and silent deletion — its own
text called a substitution glyph with a real advance "the minimum honest
fix", which is what shipped, so the section is rewritten to describe it. The
remaining gap it names (a warning at the Python export boundary) is recorded
as still open rather than quietly dropped.
Also spelled the invisible test characters as escapes: a literal U+200B or
U+FFFD in the source can be stripped by an editor or a copy-paste, silently
reducing the assertion to `_title_png("X") == _title_png("X")`.
This was referenced Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An audit of xy's customizability across eleven dimensions found one theme behind
most of the severe findings: options the WebGL client honors, the static
exporters silently drop. You style a chart in a notebook, export it for a
report, and get a different chart — no warning, and nothing surfaces until
someone reads the report.
Both columns below are
to_png()output from identical code. Only the librarychanged.
Fixed
traces and a stem is two, so four box series under a four-color
xy.theme(palette=...)all worepalette[0], and eight box series drew twocolors out of the built-in eight. Adding an outlier to one series repainted a
different one, because it changed the trace count. Marks now take one slot
per logical series (
Figure.next_series_color), and a mark given an explicitcolor=takes none — matching matplotlib's property cycle, which I checkedrather than assumed.
x_axis(format=...)/y_axis(format=...)were dropped by every staticexporter.
format="$,.0f"read$1,000,000in the browser and1.0e6inthe PNG exported from the same figure. The exporters now share the client's
grammar (
_fmt_number_spec/_fmt_time_specare ports offmtNumberSpec/fmtTimeSpec, deliberately using the same regex), andlayout()widens theleft gutter for labels that need the room instead of running them off the
canvas.
color=channel produced no legend in any static export.One trace carries N categories, and the exporters listed rows per named
trace — so
color="species"drew a single row bearing the trace's name inthe trace's constant color, beside marks painted in three. Both exporters now
expand categories the way
ChartView._legenddoes.text=onhline/vline/bands/threshold/arrowwas silently dropped,and
xy.marker()drew nothing at all. Labels were emitted fortext/calloutannotations only, andmarkerhad no geometry branch.Placement is now one helper shared by both exporters, ported from the client's
_drawAnnotationLabels; the result lands essentially pixel-for-pixel on thelive client.
0. Tick precision came from thetick step, which is meaningless on a multiplicative axis, so
0.001and0.01became two identical, wrong labels — in the client and bothexporters. Fixed on both sides; the pre-existing JS guard was dead because its
fmtLinearfallback had the same bug.color=["#ff0000", "#00ff00", "#0000ff"]factorized the hex strings, sortedthem alphabetically, and repainted them from the palette. Written-out colors
now paint themselves.
Added
xy.theme(palette={category: color})pins colors to category labels ratherthan positions. A list can only say "the first category is blue", so the same
category changes color whenever the set of categories does — most visibly across
facet panels, where a panel missing one category silently recolors the rest.
Categories the map does not name take the next default color it has not already
spent, with a warning. No wire change: the client already reads
trace.color.palette, so the mapping only reorders that list.Two deliberate calls
color=no longer consumes a palette slot. This changed oneexisting test that asserted the old trace-indexed behavior; I updated it and
its comment. matplotlib behaves the same way (verified directly).
["red", "green", "blue"]also parses ascolors, but a column holding those is ordinary category data — and
test_categorical_color_palettealready relies on that reading. Only#rrggbb/rgb()/hsl()lists are treated as literal paint, so nothingis guessed.
Verification
ruff check,ruff format, andpre-commit run --all-filesclean;ty checkback to main's 13-diagnostic baseline;abi_smokeandappend_stream_smokepass.origin/mainworktree and againstthis branch from the same script, so the pairs above differ only where
behavior changed.
30_ticks.tschange.
scripts/render_smoke_nonumpy.pytimes out on--dump-domon my machine — itdoes the same on pristine main, so it is environmental, not a regression.
Not in this PR
The audit produced 84 verified findings; this fixes 14 of them. Follow-ups are
queued for the remaining high-severity ones (datetime bars rendering zero-width,
the raster glyph atlas dropping characters,
tick_label_anglediscarded by theraster exporter,
bar(mode="stacked")being a no-op on the layered pattern thedocs teach).
Summary by CodeRabbit
{category: color}palette mappings intheme(), pinning colors by category label with stable assignments across facets and clear warnings/fallbacks for unmapped categories.format(including log subtick behavior), render categorical legends one row per category, and improve annotation/marker placement.