Match Matplotlib text weight defaults [mpl compatibility]#270
Match Matplotlib text weight defaults [mpl compatibility]#270sselvakumaran wants to merge 2 commits into
Conversation
Matplotlib 3.11's axes.titleweight, axes.labelweight and font.weight all default to "normal", and its legend titles and colorbar labels are normal too. xy rendered chart titles at 600 and axis labels, annotations and colorbar titles at 500, so every chart came out heavier than the same script under Matplotlib. Set the default to 400 in all three renderers so they agree: - _svg.py: title, axis-title and legend-title font-weight attributes - _raster.py: the title and axis-label weights fed to _native_font_emphasis - 20_theme.ts: the title, colorbar_title and annotation_label slot rules - 50_chartview.ts: the nine inline axis-label weights and the legend title (inline styles beat the slot stylesheet, so both had to change) Add the axes.titleweight and axes.labelweight rcParams to the pyplot shim so Matplotlib's own knobs work. Both publish onto chrome_styles for the browser and onto the axis style for the SVG/raster exporters, and only travel when they differ from "normal" — the value every renderer already defaults to. This changes core (non-pyplot) chart appearance: core titles now render at normal weight instead of semibold. tests/test_text_weight_defaults.py asserts the emitted weight per element in the SVG output and in the native raster command stream, and guards the TypeScript defaults at source level, since the client bundles are a generated, git-ignored artifact.
Matplotlib reference / before / after three-ups for the PDSH 04.09 cell 23 figure and a plain core chart. Temporary: remove before merge.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
i think this is a fix from another PR but should be verified - thanksgiving and christmas borders are also not correct also the margins are still an issue but should be fixed in another PR |
Summary
xy renders chart text heavier than Matplotlib everywhere. This sets the default to normal (400) in all three renderers and adds the two Matplotlib rcParams that were missing entirely:
js/src/20_theme.ts,js/src/50_chartview.ts), the SVG exporter (python/xy/_svg.py) and the native raster exporter (python/xy/_raster.py), so the three agreeaxes.titleweightandaxes.labelweightto the shim, wired to all three renderers, so explicit Matplotlib weights workStack
This is two levels deep: #243 → #255 → this PR, branched from and targeting
agent/matplotlib-text-fidelity.Branching from
mainwould have collided with #255 in the same lines. #255 makes the title/label weights overridable but leaves the defaults at 600/500, and it touches the exact expressions this PR changes (title_style.get("font-weight", 600),axis_style.get("label_font_weight", 500)). Stacking makes the two changes compose — #255 supplies the override plumbing, this PR corrects the value it falls back to — instead of conflicting.The stacking is also load-bearing for correctness, not just for merge hygiene. See Root cause.
Root cause
Two independent defects that this stack turns into one regression.
1. The defaults were never Matplotlib's. Measured against real matplotlib 3.11.1, artist-level
get_fontweight()on PDSH cell 22 (the Matplotlib reference for cell 23):axes.titleweight,axes.labelweight,figure.titleweightandfont.weightare all'normal'inmatplotlib.rcParams. xy had noaxes.titleweight/axes.labelweightkeys at all, so a user could not even opt into Matplotlib's own knobs — setting them warned and was ignored.2. The stack was about to make it worse. On
main,python/xy/_raster.pycontains no weight handling whatsoever —grep -n 'bold\|font_weight' main:python/xy/_raster.pyreturns nothing — so exported PNGs looked accidentally correct. #243 adds_native_font_emphasiswithbold = float(weight) >= 600, and the title default of 600 hits that threshold exactly. Measured on the #255 tip, before this PR:So the raster title is genuinely bold at the #255 tip where it was normal on
main. That is a regression away from Matplotlib introduced by the stack, and it is why this PR belongs inside the stack rather than after it. Axis labels escaped only because 500 < 600.js/src/20_theme.tsline 124 (annotation_label, weight 500) is untouched by all 16 open PRs.Measured before → after
Real matplotlib 3.11.1 is
normal(400) for every row.Two things worth naming from that table:
Impact on core charts — yes, these are core defaults
Core (non-pyplot)
xy.Figurecharts change appearance. These are engine defaults, not shim values, so a plainxy.chart(...)title now renders at normal weight instead of semibold, and core axis labels/annotations/legend titles lose their 500–600 weight. That is a deliberate, visible change to every existing core chart, and it is the thing to push back on if the intent is that core keeps a distinct typographic voice from the Matplotlib shim.The argument for changing core rather than only the shim: the defaults live in one place per renderer,
xy.pyplotcompiles onto the same coreFigure, and splitting them would mean core and shim disagreeing about whatstyles={"title": ...}falls back to. Heavier text stays available everywhere viastyles[slot],label_font_weight, or the rcParams.Non-goals: no change to font size, family, style, color, or layout; no change to how an explicit weight is resolved (that is #255's work, which this composes with).
Comparison images
Matplotlib reference / before / after. Both are the native PNG export path —
npm/nodeare not installed in this environment andpython/xy/static/does not exist, so I could not build the client bundles to capture the browser path. The native path is the one that regresses, so it is also the right path to show.PDSH 04.09 cell 23
Plain core chart
Real
data/births.csv(15,548 rows) through the corexy.FigureAPI rather than the shim, so the core-default change is visible on its own.Validation
Python suite, this branch vs. its pristine base, same command both sides:
The two failure sets are byte-identical (
diffof the sortedFAILED/ERRORlines is empty). All 157 are pre-existing on a pristine tree and come from the absent git-ignored JS bundles andanywidget; five further modules cannot be collected at all for the same reason and are excluded on both sides. Net: +13 passed, 0 regressions.New tests (13):
tests/test_text_weight_defaults.py(10) — per-elementfont-weightin SVG output; per-element emphasis decoded out of the native raster command stream by locating each text record from its UTF-8 payload and reading the opcode/flags byte; SVG-vs-raster agreement on the axis-label default; source-level guards on the TypeScript defaults, both the20_theme.tsslot rules and the ten inline weights in50_chartview.tstests/pyplot/test_rc_chrome_contracts.py(3) — the two rcParams default tonormaland put nothing on the wire; overrides reachchrome_styles(browser) and the axis style (SVG/raster); an explicitset_title(fontweight=)still winsNegative control on the new tests. Reverted just the source changes, kept the tests: 5 of 10 fail, covering the SVG defaults, the raster bold flag, SVG/raster agreement, the theme stylesheet and the chartview inline weights. The other 5 assert #255's override behavior and correctly stay green. So these tests fail on the defect and pass on the fix.
One existing expectation updated:
tests/test_css_mark_styles.py::test_axis_style_reaches_svg_and_native_renderersassertedfont-weight="500"for an axis label whose style sets no weight — it pinned the old default.rcParams verified end-to-end against all three renderers:
Other checks:
cargo build --release --offline— clean (no Rust change in this PR)ruff check .— all checks passed;ruff format --check .— 364 files already formattedpre-commit run --all-files— ruff check, ruff format, docs-app-codespell all passmatplotlib==3.11.1in a throwaway env, not from memoryNot verified
npmandnodeare not installed anywhere in this environment, sonode js/build.mjscannot run andpython/xy/static/does not exist. I am not claiming atscpass. As a labelled proxy I randeno check --sloppy-importsover the module graph fromjs/src/60_entries.tswithjs/tsconfig.json's flags (strict:false,noImplicitAny:false,noImplicitThis:false): clean. Negative control — injectingconst probe: number = "deliberate type error"into20_theme.tsproducedTS2322, so the proxy does have teeth. It is still a proxy, not the project's real typecheck, and the three TypeScript weight changes are unexercised at runtime here.getComputedStylein a real browser; that needs the bundles.>= 600all collapse onto the same bold glyphs in PNG output. Intermediate weights are not distinguishable there. Documented rather than fixed.Spec
spec/api/styling.md— new Chrome text weight section: the 400 default, the per-renderer table of where it lives, the opt-in forms, and the native-PNG bold approximation. Axis style table gains thelabel_font_*keys.spec/matplotlib/compat.md—rcParamsrow notes the two new keys and that they reach all three renderers; new Text weight row.spec/matplotlib/shim-todo.md— the "exporter chrome stays fixed regardless of rcParams" known-inconsistency now carves out the two weight keys, which I measured reaching single-chart SVG and PNG.No
compat-changelog.mdentry: that file scopes itself to changes in the upstream compatibility target and in the meaning of the approximation levels, and neither moves here.Coordination
A sibling agent is concurrently preparing
agent/mpl-annotation-box-text-fidelity(also stacked on #243) for annotation bbox alpha, plain-text colour and corner radius. That work overlaps this one on annotation text but on different properties — theirs is paint and box geometry, this is weight only. The only shared line is theannotation_labelslot rule in20_theme.ts.Draft because it intentionally depends on #255; retarget to
mainonce #243 and #255 land. Thepr-assets/text-weight/commit is temporary and should be dropped before merge.