Fix gallery image and legend option compatibility [mpl compatibility] - #334
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughLegend sizing and stroke rendering, image interpolation, color parsing, histogram ordering, hatch geometry, errorbar legend handling, and renderer compatibility are updated across Python and browser paths with regression tests and styling documentation. ChangesLegend behavior
Image and color compatibility
Plot and rendering compatibility
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Axes
participant LegendBuilder
participant LegendLayout
participant SVGRenderer
participant BrowserChartView
Axes->>LegendBuilder: compose validated legend options
LegendBuilder->>LegendLayout: calculate handle length and text gap
LegendLayout->>SVGRenderer: emit sized swatches and strokes
LegendBuilder->>BrowserChartView: send legend options and item styles
BrowserChartView->>BrowserChartView: apply responsive SVG and border geometry
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
b51b611 to
a916022
Compare
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
python/xy/pyplot/_axes.py (1)
2450-2470: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win**
dash_capstylealso needs to be dropped for step/drawstyle="steps-*"handles.**Line2D.set_dash_capstyle()writes the kw into the entry, and the@markdispatch forwards it straight intoxy.step(...)`, which doesn't accept it; a mutated step-based line will still fail at render time.🤖 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/pyplot/_axes.py` around lines 2450 - 2470, Update the step/drawstyle="steps-*" handle-processing path to remove dash_capstyle before the `@mark` dispatch calls xy.step(...). Ensure Line2D.set_dash_capstyle() mutations are filtered consistently with the existing unsupported keyword handling so step-based lines render without forwarding dash_capstyle.
🧹 Nitpick comments (1)
python/xy/pyplot/_axes.py (1)
2519-2537: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdaptive interpolation/resample-resolution choice is not recorded in the emitted spec.
_resolve_imshow_samplingsilently decides the interpolation kernel (nearest vs. hanning) and intermediate resample resolution; only the resulting pixel grid ships in the heatmap entry, with no field recording which method/decision was applied. As per coding guidelines, "Record every decimation and tier decision in the specification; decisions must never be silent." Since this decision is baked once into static pixel data (unlike the interactive density-tier sampling, which the JS client surfaces via "sampled N of M" badges), the practical need for recording it is less clear-cut, but worth confirming intent against the guideline.Also applies to: 7314-7357
🤖 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/pyplot/_axes.py` around lines 2519 - 2537, Update the heatmap specification emission associated with _resolve_imshow_sampling so it records the resolved interpolation method, stage, and intermediate resample dimensions returned by that function, rather than only emitting the resulting pixel grid. Apply the same metadata recording to the corresponding flow around the additionally affected section, preserving the existing sampling behavior.Source: Coding guidelines
🤖 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/pyplot/_axes.py`:
- Around line 2450-2470: Update the step/drawstyle="steps-*" handle-processing
path to remove dash_capstyle before the `@mark` dispatch calls xy.step(...).
Ensure Line2D.set_dash_capstyle() mutations are filtered consistently with the
existing unsupported keyword handling so step-based lines render without
forwarding dash_capstyle.
---
Nitpick comments:
In `@python/xy/pyplot/_axes.py`:
- Around line 2519-2537: Update the heatmap specification emission associated
with _resolve_imshow_sampling so it records the resolved interpolation method,
stage, and intermediate resample dimensions returned by that function, rather
than only emitting the resulting pixel grid. Apply the same metadata recording
to the corresponding flow around the additionally affected section, preserving
the existing sampling behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 470f495c-4ddc-4c41-834c-7e0d4de9d678
📒 Files selected for processing (12)
js/src/50_chartview.tspython/xy/_svg.pypython/xy/pyplot/_artists.pypython/xy/pyplot/_axes.pypython/xy/pyplot/_colors.pypython/xy/pyplot/_rc.pyspec/matplotlib/compat.mdtests/pyplot/test_gallery_statistics_semantics.pytests/pyplot/test_grid_legend_contracts.pytests/pyplot/test_line_legend_gallery_compat.pytests/pyplot/test_matplotlib_mismatch_regressions.pytests/test_text_weight_defaults.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/pyplot/test_line_legend_gallery_compat.py (1)
298-300: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the filtered payload, not only its trace kind.
The test confirms that the step entry becomes
kind == "line", but it never verifies thatdash_capstyleis removed before renderer dispatch. A regression could preserve the same trace kind while leaking the Matplotlib-only keyword. Inspect the serialized renderer kwargs and assert thatdash_capstyleis absent.Also applies to: 302-313
🤖 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/pyplot/test_line_legend_gallery_compat.py` around lines 298 - 300, Extend the assertions in the test around _build_chart and build_payload to inspect the serialized renderer kwargs for the first trace, and verify that dash_capstyle is absent after filtering. Keep the existing kind == "line" assertion while validating the payload passed to renderer dispatch.
🤖 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.
Nitpick comments:
In `@tests/pyplot/test_line_legend_gallery_compat.py`:
- Around line 298-300: Extend the assertions in the test around _build_chart and
build_payload to inspect the serialized renderer kwargs for the first trace, and
verify that dash_capstyle is absent after filtering. Keep the existing kind ==
"line" assertion while validating the payload passed to renderer dispatch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f2442fd-5801-4ffd-82d8-0125dabb9666
📒 Files selected for processing (3)
python/xy/pyplot/_axes.pytests/pyplot/test_line_legend_gallery_compat.pytests/test_text_weight_defaults.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_text_weight_defaults.py
- python/xy/pyplot/_axes.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@python/xy/pyplot/_artists.py`:
- Around line 966-972: Update ErrorbarContainer.set_label to store str(value) in
the name field, matching Artist.set_label’s normalization while preserving the
existing _touch() call.
In `@python/xy/pyplot/_plot_types.py`:
- Around line 1979-1986: Update the extent/span computation used by ring() to
track edge-span and value-span separately, then select the span paired with each
rectangle axis according to orientation. In the horizontal branch, use the value
span for rect_width and the edge span for rect_height; retain the existing
edge-to-width and value-to-height pairing for vertical orientation. Apply this
consistently to the related hatch sizing logic around ring() and the non-fill
horizontal path.
🪄 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: 22dc6cba-8b50-495f-8e02-ce9d5e9ad8f9
📒 Files selected for processing (15)
js/src/50_chartview.tspython/xy/_raster.pypython/xy/_svg.pypython/xy/pyplot/_artists.pypython/xy/pyplot/_axes.pypython/xy/pyplot/_plot_types.pyspec/api/styling.mdspec/matplotlib/compat-changelog.mdspec/matplotlib/compat.mdtests/pyplot/test_gallery_hist_errorbar_compat.pytests/pyplot/test_gallery_statistics_semantics.pytests/pyplot/test_grid_legend_contracts.pytests/pyplot/test_line_legend_gallery_compat.pytests/pyplot/test_matplotlib_mismatch_regressions.pytests/test_text_weight_defaults.py
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/pyplot/test_grid_legend_contracts.py
- spec/matplotlib/compat.md
- tests/pyplot/test_matplotlib_mismatch_regressions.py
- js/src/50_chartview.ts
# Conflicts: # python/xy/pyplot/_axes.py
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 `@python/xy/pyplot/_colors.py`:
- Around line 371-388: The is_color_like function currently lets OverflowError
escape when resolve_rgba processes non-finite RGB(A) channels. Update the
exception handling around resolve_rgba to treat OverflowError like TypeError and
ValueError, returning False for such inputs, and add a regression test covering
non-finite RGB(A) values.
🪄 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: cb91c778-4a76-45f2-a72d-8653c143c25a
📒 Files selected for processing (13)
python/xy/_raster.pypython/xy/_svg.pypython/xy/pyplot/_artists.pypython/xy/pyplot/_axes.pypython/xy/pyplot/_colors.pypython/xy/pyplot/_fmt.pypython/xy/pyplot/_plot_types.pyspec/api/styling.mdtests/pyplot/test_axes_charts.pytests/pyplot/test_fmt.pytests/pyplot/test_gallery_hist_errorbar_compat.pytests/pyplot/test_gallery_statistics_semantics.pytests/pyplot/test_launch_compat.py
🚧 Files skipped from review as they are similar to previous changes (7)
- python/xy/_svg.py
- tests/pyplot/test_gallery_statistics_semantics.py
- python/xy/pyplot/_artists.py
- spec/api/styling.md
- python/xy/_raster.py
- python/xy/pyplot/_plot_types.py
- python/xy/pyplot/_axes.py
3 participants
Summary
This is the first grouped compatibility batch after #280/#281. It fixes exact-gallery execution regressions and blockers without adding review assets or compatibility-ledger edits to the PR:
legend(handlelength=...)andhandletextpaduse Matplotlib legend-font units in browser, SVG, and PNG output;Line2D.set_dash_capstyle("round")survives through the fixed-round renderer path;imshow(interpolation="auto")/"antialiased"use Matplotlib 3.11's adaptive nearest/Hanning and data/RGBA-stage rules;limegreenwork for colormap extremes;Exact gallery evidence
The PNGs live on an immutable review-only evidence commit, not in this PR or
main.Dashed-line legend handle geometry
Adaptive image interpolation
Histogram legend regression
Verification
Current integration state:
4c26d97, current headfd31f02;git diff --checkpasses;compat.md, compatibility changelog, shim ledger,pr-assets, or evidence PNG changes in the net PR diff;The exact gallery sweep predates the current-main merge and is retained as visual evidence, not presented as a fresh post-merge run:
a2ac10e: 79/88 paired examples, 136 paired figures;a916022: 83/88 paired examples, 155 paired figures;image_antialiasing.py: 11/11 figures, 4 near / 7 similar / 0 divergent;The expensive exact-gallery/browser capture was not rerun after the conflict-free current-main merge. Fresh GitHub Actions are the final integration gate.
Out of scope for this PR
These selected-gallery areas remain separate work:
fill_spiral.py— general non-simple polygon filling;pie_and_donut_labels.py— annotation/pie geometry;log_demo.py— separate minor log ticks/grid plus base/nonpositive scale options;time_series_histogram.py— logarithmic scalar normalization and automatic colorbar layout;subplots_adjust.py— explicitcolorbar(cax=...)placement.This PR does not claim blanket pixel equivalence for the full gallery.
Summary by CodeRabbit
New Features
interpolation="auto"and improved image resampling behavior.Bug Fixes