Skip to content

Fix clipped colorbar labels [mpl compatibility] - #268

Closed
sselvakumaran wants to merge 2 commits into
mainfrom
agent/fix-colorbar-label
Closed

Fix clipped colorbar labels [mpl compatibility]#268
sselvakumaran wants to merge 2 commits into
mainfrom
agent/fix-colorbar-label

Conversation

@sselvakumaran

Copy link
Copy Markdown
Contributor

Summary

  • render a vertical colorbar's label rotated 90° counter-clockwise beside the bar in native PNG, instead of horizontally above it
  • stop clipping the label off the top edge of the export canvas
  • share one label baseline between the SVG and native PNG exporters so the two static paths cannot drift
  • document the colorbar label's side/rotation contract in spec/api/styling.md, the compatibility matrix, and the compatibility changelog

Why

PDSH ch. 04.05 sets a colorbar label three times — hist2d + set_label, hexbin(label=...), and imshow + set_label. Matplotlib draws that label rotated a quarter turn alongside the bar and centered on it. xy drew it as a horizontal title above the bar, with the top of every glyph sheared off at the canvas edge. Nothing raised, so no corpus test caught it: the label text was present, just in the wrong place and partly outside the image.

Matplotlib reference, before, and after

Root cause

python/xy/_raster.py:2103 (pre-fix) placed the vertical label at (x, plot.y - 5) with no rotation flag, justified by this comment:

# The native text primitive does not rotate; a compact label above the
# bar remains legible and, crucially, stays inside the export canvas.

Both halves were wrong. The native text primitive does rotate — TEXT_ROTATED / TEXT_ROTATED_CW (src/raster.rs:1188), surfaced as _TEXT_ROT_CCW / _TEXT_ROT_CW and already used for rotated y-axis titles. And the chosen spot does not stay inside the canvas: plot.y is the top margin, so a baseline at plot.y - 5 puts the glyph ascent (~0.78em) above row 0. Decoding the exported PNG shows label ink in device rows 0–9, i.e. clipped.

The label now emits at x + width + 38 — the SVG exporter's existing baseline — centered along the bar with _TEXT_ROT_CCW, inside the extra right-margin room layout() already reserves whenever a vertical colorbar has a label.

Rotation fidelity

Quarter turns are exact in the native rasterizer, so this label is fully faithful there. #243's disclosure still stands for arbitrary angles — those render as upright glyphs in native PNG — but a colorbar label is always 0° or 90°, so it never hits that limit. No new fidelity caveat.

Impact

  • Native PNG only. The browser client (js/src/50_chartview.ts:1927, writing-mode: vertical-rl) and the SVG exporter (_svg.py, rotate(-90 …)) were already correct: right side, rotated, on canvas. Both are deliberately untouched, so no unbuildable TypeScript ships in this PR (see Validation).
  • Core colorbars change too. This is renderer-level, not shim-level: a composition-API colorbar built with xy.colorbar(title=...) renders its vertical label through the same code, so its PNG output changes the same way — from a clipped title above the bar to a rotated label beside it. That is the intended appearance in both surfaces, and it is now specified rather than incidental.
  • Horizontal colorbars are untouched in all three renderers; their label was already upright and centered below the bar.

Scope

Colorbar placementlocation, anchor, shrink, minor ticks, and masked-sample domain inference — belongs to #246, which documents it under ### Colorbar placement and ticks. This PR touches none of it: no placement option is read or added, and the bar's own x/y/width/height are computed exactly as before. My spec additions live in a separately named ### Colorbar label orientation subsection and a new compatibility-matrix row, so they should not collide with #246's text.

Honest limits of the comparison image: the hist2d row's plot body is still wrong in xy (coarse blocks over a 0–1 domain rather than a 30-bin 2-D histogram). That is an independent, separately owned hist2d binning/domain defect, unaffected by this PR and not claimed as fixed here. The hexbin and imshow rows are visually correct after this change.

Deliberate follow-ups

Two more gaps from the same PDSH audit are left out rather than bundled — neither is a few lines, and neither shares this PR's theme:

  • from xy.pyplot import Triangulation raises ImportError (04.12 cell 17). xy exposes a triangles= shorthand but no Triangulation class; a faithful shim needs Matplotlib's implicit Delaunay triangulation when triangles is omitted, which is real work. Worth noting that fixing the import alone would not make the cell render — it then hits xy's deliberate 3-D rejection.
  • axhline(marker=...) raises TypeError: unsupported keyword(s): marker, reached through seaborn's FacetGrid.map(plt.axhline, ...) (04.14 cell 31). Matplotlib accepts and applies it. This is a different subsystem (span-line kwarg threading) from colorbar label geometry.

Validation

New tests assert real emitted geometry — the raster command stream and SVG coordinates — not screenshots:

  • test_native_vertical_colorbar_label_is_rotated_beside_the_bar_inside_the_canvas — anchor is exactly 1 | _TEXT_ROT_CCW, baseline is outboard of every tick label, y is the bar's midpoint, and the glyph box stays under the canvas width
  • test_native_vertical_colorbar_label_leaves_the_canvas_edges_unpainted — decodes the exported PNG and asserts no ink on any border row/column (the clipping symptom itself)
  • test_native_horizontal_colorbar_label_stays_upright_below_the_bar — guards against over-rotating
  • test_svg_vertical_colorbar_label_is_rotated_beside_the_bar_inside_the_canvas — asserts rotate(-90 …), text-anchor="middle", containment, and that the native exporter emits the identical baseline
  • test_colorbar_set_label_renders_rotated_beside_the_bar_in_both_exports — the PDSH path end to end through xy.pyplot
$ ruff check .
All checks passed!

$ ruff format --check .
366 files already formatted

$ pre-commit run --all-files
ruff check...............................................................Passed
ruff format..............................................................Passed
docs app codespell.......................................................Passed

$ ty check python/xy/_raster.py
All checks passed!

$ pytest tests/test_png_export.py tests/test_svg_export.py \
         tests/test_declarative_colorbar.py tests/pyplot/test_pdsh_gap_features.py
2 failed, 157 passed, 3 skipped

Full suite, before and after this branch:

baseline (branch point 119a1ce):  167 failed, 1810 passed, 76 skipped
with this branch:                 167 failed, 1815 passed, 76 skipped

comm over the two sorted FAILED lists is empty in both directions — an identical failure set, +5 passing, which are exactly the five new tests.

Not verified

Reported precisely rather than glossed:

  • Those 167 failures (including the 2 above) are pre-existing and unrelated: this environment has no python/xy/static/ bundles, and npm/node are not installed, so every test that reads the JS client fails on a pristine tree. I baselined the pristine tree to confirm the set is unchanged rather than attributing any of them to this change. Five further modules cannot even be collected without the bundles and were excluded from both runs.
  • Consequently node js/build.mjs was not run and the TypeScript is not typechecked or bundled here. This PR deliberately contains no js/ changes, so nothing in the render client is left unverified by that gap — but I cannot claim a JS build pass the way Fix pie labels and boxed text [mpl compatibility] #243 and Fix colorbar gallery placement and domains [mpl compatibility] #246 do.
  • The 13 remaining ty check diagnostics across the package are pre-existing (10 are unresolved imports from the same missing bundles); none are in the files this PR touches, and ty check on python/xy/_raster.py is clean.
  • Matplotlib reference panels were rendered with 3.11.1 in a throwaway venv. The before panels are real pre-fix code — the packaged _raster.py from the branch point, rendered through PYTHONPATH — not a monkeypatch.

The native raster exporter drew a vertical colorbar's label horizontally
above the bar at `plot.y - 5`, so the glyph ascent overflowed the canvas
top edge and the text was clipped. Matplotlib rotates the label 90° CCW
and centers it alongside the bar, outboard of the tick labels.

The placement was chosen on the assumption that the native text primitive
could not rotate. It rotates in quarter turns (TEXT_ROTATED /
TEXT_ROTATED_CW, already used for rotated axis titles), so 90° is exact
here — only arbitrary text angles still fall back to upright glyphs.

The label now shares the SVG exporter's baseline (`x + width + 38`), so
the two static paths cannot drift, and it sits inside the extra
right-margin room `layout()` already reserves for a labeled colorbar.
Browser and SVG output are unchanged; both were already correct.

Clears the three visually wrong PDSH ch. 04.05 cells (hist2d, hexbin and
imshow colorbars with labels). Composition-API colorbars built with
`xy.colorbar(title=...)` share the renderer, so their vertical labels
change in PNG the same way; horizontal labels are untouched.
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 80a7f152-7599-41a5-8a39-98d0e2ba04d6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-colorbar-label

Comment @coderabbitai help to get the list of available commands.

@codspeed-hq

codspeed-hq Bot commented Jul 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 102 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing agent/fix-colorbar-label (07447ac) with main (119a1ce)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant