Fix mesh, image, and distribution autoscale [mpl compatibility] - #269
Fix mesh, image, and distribution autoscale [mpl compatibility]#269sselvakumaran wants to merge 3 commits into
Conversation
The pyplot shim materializes its own view before compiling to a core Figure, so `Axes._iter_entry_arrays` has to recognize every entry shape the shim can emit. It probed top-level `x`/`y` plus a small factory table, and three shapes keep their coordinates inside `kwargs` instead: `@mark`/`heatmap` (hist2d, uniform-grid pcolormesh, specgram), `ecdf`, and `box`. Those axes scanned as dataless, so `_build_chart` pinned them to the empty `(0, 1)` view and clipped the geometry — a 30x30 hist2d rendered roughly 3x1 of its bins. The pre-existing `kind == "heatmap"` branch did not cover hist2d: that branch matches imshow's entry shape, which is a plain `heatmap` kind carrying an explicit `extent`. hist2d registers an `@mark` kind with factory `heatmap` and no `extent` at all, so it fell through to the `@mark` factory table, which had no `heatmap` row. Mirror the core rather than re-deriving the geometry: heatmap spans go through `Figure._heatmap_axis_positions`/`_cell_edges`, which is what recovers the original bin edges from the centers hist2d stores, and box spans reuse `marks._distribution_stats` for whiskers and fliers. Matplotlib gives these artists sticky edges, so their limits carry no margin. The engine knows only the zero-baseline anchor, so derive sticky edges for mesh, image, and ECDF entries and, when both ends are pinned, ship a materialized `domain` instead of a `margin` — the two never combine. One-sided rectangle baselines keep the `margin` and stay anchored by the engine. hexbin, contour/contourf, imshow, non-uniform hist2d/pcolormesh, tripcolor, violinplot, errorbar, and step already scanned correctly.
TEMPORARY — for PR review only, to be removed before merge. Three-up render of the PDSH 04.05 `plt.hist2d` cell: Matplotlib 3.11.1 reference, xy.pyplot before (30x30 bins computed, view pinned to the dataless 0..1 box so roughly 3x1 of them are visible), and xy.pyplot after.
|
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 |
…against The limits in the new changelog entry were compared against Matplotlib 3.11.1, not 3.11.0.
Merging this PR will degrade performance by 21.79%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | test_select_lasso_message_1m |
88.8 ms | 122.2 ms | -27.32% |
| ❌ | test_pyramid_compose |
9.3 ms | 11 ms | -15.85% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing agent/fix-pyplot-hist2d-autoscale (fc01a2b) with agent/fix-pyplot-autoscale-margins (5c7418e)
Footnotes
-
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. ↩
1 participant
Summary
kwargsinstead of as top-levelx/y:@mark/heatmap(hist2d, uniform-grid pcolormesh, specgram),ecdf, andboxFigure._heatmap_axis_positions/_cell_edges, box spans throughmarks._distribution_statstests/pyplot/test_mesh_autoscale_regressions.py(13 cases) covering every factory touchedEntirely inside the shim (
python/xy/pyplot/_axes.py). No core change, so no non-pyplot chart changes behavior — the declarative API already put the correct span on its heatmap traces; only the shim's own pre-build view scan was wrong.Root cause
Axes._iter_entry_arrays(python/xy/pyplot/_axes.py:2782) is the scanner that feeds_axis_is_dataless. It probes top-levelentry["x"]/entry["y"]plus a small@markfactory table.hist2dregisters neither:Its bin centers live in
mark_kwargs["x"]/["y"], so the top-level probe found nothing and the factory table had noheatmaprow. The axis scanned as dataless, and_build_chart(_axes.py:4886) then materialized(0, 1)over the real geometry.Why the existing heatmap branch did not cover it.
_axes.py:2805already had:That is
imshow's entry shape — a plainheatmapkind carrying an explicitextent(_axes.py:2134).hist2dregisters an@markkind with factoryheatmapand noextentat all, so it never reached that branch; it fell into the@markarm and matched no factory. Two unrelated entry shapes share the word "heatmap", which is what made this look covered. The new code is afactory == "heatmap"arm inside the@markblock, with a comment naming the distinction so the next reader does not re-conflate them.Getting the span right needs the cell edges, not the centers
hist2dstores.marks.heatmapreconstructs them withFigure._cell_edges, extending half a cell past the first and last center — which is exactly what recovers the original bin edges. Calling that helper rather than re-deriving it is what makesget_xlim()land on the bin edges to the last float.Sticky edges
Measured against Matplotlib 3.11.1:
hist2d/pcolormeshset sticky edges, so their limits are the outer cell edges with no margin.The engine knows only the zero-baseline rectangle anchor (
Figure._zero_baseline_anchor), so the shim resolves this itself:_entry_sticky_edgesgains mesh/image/ECDF spans, and_fully_sticky_domainships a materializeddomainin place of amarginwhen both ends are pinned.marginanddomainnever combine, perspec/design/pan-and-zoom-configuration.md§9. A one-sided sticky edge is the bar/histogram baseline and is deliberately excluded — those keep theirmarginand stay anchored by the engine (regression-tested).Impact
PDSH
pdsh_04_05_histograms_and_binnings.ipynb, theplt.hist2dcell:get_xlim()(0.0, 1.0)(-4.1515520873, 3.6958252221)(-4.1515520873, 3.6958252221)get_ylim()(0.0, 1.0)(-5.4578953516, 5.3459615454)(-5.4578953516, 5.3459615454)Honest scope of that image: the geometry and axis limits now match. Two differences remain in the after panel that this PR does not fix and does not claim — the colorbar's "counts in bin" label is clipped at the top of the colorbar rather than rotated alongside it, and the x tick density is higher than Matplotlib's. Both are independent of autoscale.
Every factory audited
I swept every factory the shim can emit, not just the reported one. Verdicts on this branch's base:
hist2d(uniform bins)hist2d(range=...)heatmapfactory)pcolormesh, uniform gridheatmapfactory)pcolormesh, regular 2-DX/Yheatmapfactory)specgramheatmapfactory)ecdfboxplothexbinx/yandargscontour/contourfcontourbranchimshow, with and withoutextentkind == "heatmap"branch; gained sticky edges here so it no longer takes a marginhist2d/pcolormesh, non-uniform binstriangle_meshtripcolor/triangle_meshviolinplotsegmentsentrieserrorbarstepstairs,stem,fill_betweenstem) and #252 (stairs,area+ baseline) — deliberately untouched here to avoid duplicating those stacksCaveat found while auditing boxplot
boxplot's value axis is now exact against Matplotlib on both settings:Its category axis is a separate, pre-existing defect this PR does not fix: with
positionsomitted,_plot_types.py:1876forwardspositions=None, so the core places boxes on 0-based ordinals, where Matplotlib uses 1-based positions (the siblingviolinplotpath at_plot_types.py:1992already corrects this withnp.arange(1, count + 1)). Soget_xlim()is(-0.38, 1.38)against Matplotlib's(0.5, 2.5). The scanner change here faithfully mirrors where the core actually draws; fixing the placement is a one-line registration change with its own rendering consequences and belongs in its own PR. Documented as a known deviation inspec/matplotlib/compat.md.Spec
spec/matplotlib/shim-todo.md— corrected a now-false claim: it stated sticky edges were "intentionally out of scope because xy artists do not expose sticky-edge metadata". They are now derived from the entry list instead of from artist metadata.spec/design/pan-and-zoom-configuration.md§9 — new row in the margin/domain contract table for the both-ends-sticky case, and why it ships adomainrather than amargin.spec/matplotlib/compat.md—hist2d/ecdf,imshow/pcolormesh, andboxplotrows record the new limit behavior and the boxplot position deviation.spec/matplotlib/compat-changelog.md— dated entry naming which factories changed and which were already correct (dossier §28: autoscale decisions recorded, never silent).Validation
Native core rebuilt in-worktree first (
cargo build --release --offline); the prebuilt dylib reports ABI 37 against a wrapper expecting 39 and will not load.Parity numbers in this description were measured against a real Matplotlib 3.11.1 in a throwaway venv, not asserted from memory.
Unverified: no JS/TS change is included, so nothing needed the bundles — but the ~180 bundle-dependent tests could not run here either way, and I baselined rather than blamed them. No browser/pixel smoke run.
Sequencing
Branched from and targets
agent/fix-pyplot-autoscale-margins(#240), which rewrites_entry_extent/ the auto-domain logic in this same file; a fix written againstmainwould conflict. Rebased onto #240's current head5c7418e(its in-flight commit) and neither rebased nor force-pushed that branch. Stacked alongside #253 and #256 on the same base.Commits:
0eeaf61is the fix,fc01a2bcorrects the Matplotlib revision named in the changelog entry, and198eb17adds the comparison asset and is temporary — drop it before merge.