Fix scatter histogram layouts built with inset axes - #363
Draft
FarhanAliRaza wants to merge 1 commit into
Draft
Conversation
📝 WalkthroughWalkthroughChangesIndependent inset axes
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ParentAxes
participant Figure
participant InsetAxes
Caller->>ParentAxes: call inset_axes(bounds, sharex, sharey)
ParentAxes->>Figure: convert bounds and call add_axes()
Figure-->>ParentAxes: return independent InsetAxes
ParentAxes->>InsetAxes: store parent-relative metadata
ParentAxes->>Figure: share requested axis properties
Figure->>InsetAxes: resolve rectangle from parent position
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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.
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/_axes.py`:
- Around line 5249-5274: Update the tight-layout handling around the
inset-family logic to recompute fitting on every layout solve, not only when
_inset_layout_base is unset. Preserve the parent’s original solved allocation
during layout resolution before aspect adjustment, then use that allocation as
the base for _figure_rect and recalculate the family bounds and fitted rectangle
each solve. Avoid using get_position() as the stored base, preventing
aspect-adjusted geometry from being applied twice.
🪄 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: 869abe8c-368a-4828-a1f1-44961dbf5626
📒 Files selected for processing (5)
python/xy/pyplot/_axes.pypython/xy/pyplot/_mplfig.pyspec/matplotlib/compat-changelog.mdspec/matplotlib/compat.mdtests/pyplot/test_frame_geometry.py
Comment on lines
+5249
to
+5274
| if self.figure._layout_options.get("engine") == "tight": | ||
| # Constrained layout must leave figure room for marginal axes | ||
| # whose parent-relative coordinates extend above/right (or below/ | ||
| # left) of the main panel. Preserve the first solved parent box | ||
| # and fit the whole inset family inside that allocation. | ||
| if self._inset_layout_base is None: | ||
| self._inset_layout_base = self.get_position().bounds | ||
| family = [ | ||
| child._inset_bounds | ||
| for child in self.figure.axes | ||
| if child._inset_parent is self and child._inset_bounds is not None | ||
| ] | ||
| min_x = min([0.0, *(item[0] for item in family)]) | ||
| min_y = min([0.0, *(item[1] for item in family)]) | ||
| max_x = max([1.0, *(item[0] + item[2] for item in family)]) | ||
| max_y = max([1.0, *(item[1] + item[3] for item in family)]) | ||
| base_x, base_y, base_w, base_h = self._inset_layout_base | ||
| fitted_w = base_w / (max_x - min_x) | ||
| fitted_h = base_h / (max_y - min_y) | ||
| self._figure_rect = ( | ||
| base_x - min_x * fitted_w, | ||
| base_y - min_y * fitted_h, | ||
| fitted_w, | ||
| fitted_h, | ||
| ) | ||
| self._invalidate() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Recompute inset-family fitting as part of every tight-layout solve.
This runs only when an inset is created while tight layout is already active. Calling fig.tight_layout() after creating insets—or invalidating a constrained layout later—does not refit the family. Also, get_position() captures the aspect-adjusted active box, then assigning it to _figure_rect causes adjustable-box aspect handling to be applied again.
Store/recompute the parent’s original solved allocation during layout resolution, then derive the fitted parent rectangle from that allocation on every solve.
🤖 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 5249 - 5274, Update the tight-layout
handling around the inset-family logic to recompute fitting on every layout
solve, not only when _inset_layout_base is unset. Preserve the parent’s original
solved allocation during layout resolution before aspect adjustment, then use
that allocation as the base for _figure_rect and recalculate the family bounds
and fitted rectangle each solve. Avoid using get_position() as the stored base,
preventing aspect-adjusted geometry from being applied twice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Axes.inset_axesstacked inset artists into the parent axes and failed to render parent-relative marginal panels correctly (regression reported in scatter + histogram example using mplinsert_axesdoes not work #354).Description
Axes.inset_axesto create a free-form figure panel by converting the parent-relativeboundsto a figure rectangle and callingFigure.add_axes, rather than projecting inset artists into the parent data space, and preserve staticsharex/shareylinks viaFigure._share_subplot_axes._inset_parent,_inset_bounds,_inset_layout_base) and makeFigure._axes_rectreturn parent-relative positions for inset axes so they follow aspect corrections and layout solves.Figureuses thetight/constrainedengine, reserve space for inset families that extend outside the parent by fitting the inset family into the solved parent allocation so marginal panels are contained instead of clipped or stacked.sharex/shareystatic sharing, assert marginal axes are independent panels and that constrained layout reserves room for out-of-bounds insets.Axes.inset_axessemantics.Testing
uv run pytest tests/pyplot/test_frame_geometry.py -qand the new tests passed (24 passed).uv run pytest tests/pyplot/test_layout_text_parity_fixes.py tests/pyplot/test_gallery_layout_api_regressions.py -q(44 passed).uv run pytest tests/pyplot -q: 1,099 passed, 67 skipped, with one pre-existing performance guardrail failing (unrelated timing assertion intest_pyplot_build_tracks_declarativefor 10k rows).uv run ruff check .anduv run ruff format --check .(passed); repository hooks viapre-commitcould not be executed because the environment could not fetchpre-commitfrom PyPI./tmp/scatter_hist_xy.pngand visually inspected that the top and right histograms render as independent marginal panels rather than stacked onto the central scatter.Summary by CodeRabbit
New Features
Bug Fixes
Documentation