Complete axes outer-label and margin semantics [codex bughunt] - #355
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 (2)
📝 WalkthroughWalkthroughChangesAxes behavior updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Axes.autoscale_view
participant Axes._set_tight_domains
participant _build_chart_uncached
Caller->>Axes.autoscale_view: provide tight state
Axes.autoscale_view->>Axes._set_tight_domains: update tight domains
Axes._set_tight_domains->>_build_chart_uncached: rebuild chart limits
_build_chart_uncached-->>Caller: return updated limits
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
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)
4795-4838: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
tight=Trueshould not freeze autoscaling
autoscale(..., tight=True)currently writes a fixeddomainand adds the axis to_explicit_domains, so later data updates andmargins()calls stop affecting that axis. Matplotlib keeps autoscaling active here; only the margin state should change, andautoscale_view(tight=True)should still honor the configured margins instead of snapping to_entry_extent.🤖 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 4795 - 4838, Update autoscale so tight=True only updates the tight/margin state and does not write a fixed domain or add axes to _explicit_domains; preserve active autoscaling for subsequent data updates and margins() calls. In autoscale_view, ensure tight=True uses the configured margins rather than snapping limits to _entry_extent, while retaining explicit-domain behavior for disabled autoscaling.
🤖 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 4795-4838: Update autoscale so tight=True only updates the
tight/margin state and does not write a fixed domain or add axes to
_explicit_domains; preserve active autoscaling for subsequent data updates and
margins() calls. In autoscale_view, ensure tight=True uses the configured
margins rather than snapping limits to _entry_extent, while retaining
explicit-domain behavior for disabled autoscaling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c94a4e7-1f76-429d-8e03-ba40a4144b16
📒 Files selected for processing (2)
python/xy/pyplot/_axes.pytests/pyplot/test_axes_outer_label_margin_parity.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
06c7aed to
e47d63a
Compare
e47d63a to
3f7baf9
Compare
3f7baf9 to
6de8b45
Compare
|
Restacked cleanly onto the updated #351 head; the PR-relative stable patch ID is unchanged. The earlier live-autoscale finding remains fixed in the current layer. Full repository hooks, Ruff, formatting, and @coderabbitai review |
|
The PR description image shows a three-column visual comparison: “Matplotlib 3.11 reference”, “XY before”, and “XY after.” Each column contains four plots titled Experiment 1–4, with colored line-and-circle-marker series (blue, red, green, purple), x-axis label “Elapsed time (hours)”, and y-axis label “Measured response (units)”. The “XY after” plots visually align with the Matplotlib reference in label placement and subplot presentation, while retaining the same data trends as “XY before.”
✅ Action performedReview finished.
|
1 participant
Summary
Axes.label_outer(remove_inner_ticks=False)using the complete GridSpec span, including top/right label positions and optional inner-tick removalAxes.margins()return the configured margins and reject mixed positional/keyword values without mutating statetight=True/False/Noneautoscale state across margins, live autoscale requests, disabled axes, andaxis(...)modesadd_axes()instances unchangedWhy
label_outer()was a no-op, so scientific subplot grids retained repeated interior labels and crowded their gutters. The margin API also differed in getter, argument-conflict, and round-number autoscale behavior. These are general shim semantics, not example-specific fixes.This is stack layer 2, based on #351. It should be reviewed and merged after #351.
Visual acceptance
Exact comparison: Matplotlib 3.11.0 / XY before
074122d9b702fa16bdf8b78b318eb5b483e869b3/ XY after06c7aeddb413f5fed49918cff26758797bde78aa.The reproduction scripts and individual panels are on the evidence-only branch at commit
22c5de9170a7f080c6a8751db35c153bae15ac09; no review assets are included in this product diff.Verification
uv run --with pre-commit pre-commit run --all-filesuv run ruff check .uv run ruff format --check .git diff --checkNo browser process was used for this shim-only layer.
Summary by CodeRabbit
New Features
margins()can now report current margin settings as well as update them.axis("tight"),axis("image"), and related modes.Bug Fixes