Honor authored plot and text styles [codex bughunt] - #351
Conversation
|
Warning Review limit reached
Next review available in: 10 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 (10)
📝 WalkthroughWalkthroughUpdates Matplotlib-compatible text color and rotation handling, refactors bar errorbar containers and legend semantics, and adds SVG marker stroke opacity support with regression tests. ChangesText styling parity
Bar errorbar parity
SVG marker opacity
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Merging this PR will not alter performance
Comparing Footnotes
|
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/_artists.py`:
- Around line 999-1002: Update ErrorbarContainer.set_label so a None value
remains None instead of being converted to the string "None". Preserve string
conversion for non-None values, and ensure both _mpl_container_label and
kwargs["name"] use the cleared None value appropriately.
🪄 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: c57bc18f-743d-4c23-bf5c-01560cf958fa
📒 Files selected for processing (10)
python/xy/_svg.pypython/xy/pyplot/__init__.pypython/xy/pyplot/_artists.pypython/xy/pyplot/_axes.pypython/xy/pyplot/_mplfig.pypython/xy/pyplot/_plot_types.pytests/pyplot/test_bughunt_authored_style_parity.pytests/pyplot/test_dark_background_text_parity.pytests/pyplot/test_gallery_text_pie_compat.pytests/test_svg_annotation_marker_opacity.py
074122d to
812e384
Compare
812e384 to
01a2482
Compare
01a2482 to
59f07bb
Compare
|
Restacked cleanly onto the updated #340 head; the PR-relative stable patch ID is unchanged. Full repository hooks, Ruff, formatting, and @coderabbitai review |
|
The PR description image shows the bar-chart comparison: “XY after” restores the solid, authored bar styling relative to “XY before”, while retaining the red error bars and readable “notes stay visible” / “peak” annotations. It also shows the opacity comparison where “XY after — alpha on fill + outline” makes the cyan ring translucent on both its fill and stroke, matching the displayed
✅ Action performedReview finished.
|
Summary
This is the first bug-hunt PR stacked directly on #340 (
agent/gallery-layout-text-round2). It fixes a coherent set of cases where user-authored styling was accepted but lost, mutated, or exported inconsistently.Bar and errorbar styling
bar()/barh()error geometry through the shim's canonicalerrorbar()path;ecolor,elinewidth/linewidth, and point-sized caps;error_kwprecedence over direct bar arguments;error_kwmappings instead of mutating them;ErrorbarContainerbefore theBarContainer, preserve public labels, and keep the default_nolegend_label out of rendered automatic legends;Text and axis labels
"vertical", and"horizontal"axis-label rotations with the renderer-coordinate sign Matplotlib expects;rotation=None, so omission preserves prior state andNoneresets it;text.color="white"todark_background;suptitle(color=None)like the Matplotlib default and preserve explicit overrides.SVG marker paint
Why
These were fidelity bugs rather than palette differences: valid Matplotlib inputs were silently discarded, a caller-owned mapping was modified, dark-background labels became unreadable when exported after leaving a style context, a private
_nolegend_label leaked into the visible chart, and SVG fill/stroke opacity disagreed.Visual comparisons
Review-only artifacts live on immutable evidence commit
15620da82939f25be38c53a174f819a027b1c0dc; they are not in this PR diff.Matplotlib / XY before / XY after
The same script now preserves red error bars and width, visible dark-theme text/annotation defaults, and the authored x-label rotation. The comparison uses an explicit
labelpadso it demonstrates rotation forwarding without conflating the separate general rotated-label-layout issue.SVG marker opacity
Before,
opacity=0.22affected the purple fill but left the cyan outline opaque. After, both paints honor the authored alpha.Verification
143 passedacross the focused and adjacent bar/errorbar, legend, text/theme, artist-state, chrome, and exporter regressions;92 passedSVG, figure-text, and multiline-layout exporter pass;437 filesclean;git diff --checkpassed;No browser or Playwright process was launched locally. No compatibility ledger, changelog, shim-todo, docs spec, or PR asset file is part of the product diff.
Explicitly deferred
The existing compact public capline artist graph is not changed here. A partial two/four-capline repair was rejected during review because mixed
xerr+yerrand limit-carets also require correctLine2Dordering and one barline collection per error axis. That broader public-artist repair should land as a complete later stack layer rather than a misleading partial fix.Summary by CodeRabbit