Skip to content

Commit 00fe299

Browse files
committed
fix(layout): shift each strip past its own panel's axis
For strip_placement="outside" the strips of a group were all shifted by the plot-edge axis band, so a panel that draws no axis on the strip's side (fixed scales, multi-row facets) got its strip floated outward to clear an axis that is not there — and the interior gullies were sized for strips that hug their panels, letting shifted strips overlap the panel below. Each strip now clears exactly the ticks, labels and panel-facing margin its own panel draws on that side (PlotLayoutItems.strip_shift); the side-level strip_band_offset keeps only its axis member, used for strip_placement="inside".
1 parent 22a8fd3 commit 00fe299

3 files changed

Lines changed: 103 additions & 41 deletions

File tree

plotnine/_mpl/layout_manager/_plot_layout_items.py

Lines changed: 81 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -348,21 +348,65 @@ def strip_text_y(self, position: StripPosition) -> float:
348348

349349
return max(widths) if widths else 0
350350

351-
def axis_ticks_x_max_height_at(
352-
self, location: AxesLocation, side: str
353-
) -> float:
351+
def strip_shift(self, st: StripText) -> float:
352+
"""
353+
Outward shift of one strip past its own panel's axis, figure space
354+
355+
For `strip_placement="outside"` a strip sits beyond the ticks,
356+
tick labels and panel-facing text margin that its own panel draws
357+
on the strip's side, separated by the strip_switch_pad. A panel
358+
that draws no axis on that side keeps its strip next to the
359+
panel, so within one facet the strips of axis-bearing panels
360+
shift while the others do not.
361+
"""
362+
theme = self.plot.theme
363+
if theme.getp("strip_placement") != "outside":
364+
return 0
365+
side, ax = st.position, st.ax
366+
W, H = theme.getp("figure_size")
367+
if side in ("top", "bottom"):
368+
g, dim = "x", H
369+
text = self.axis_text_x_max_height(ax, side)
370+
ticks = self.axis_ticks_x_max_height(ax, side)
371+
facing = "b" if side == "top" else "t"
372+
else:
373+
g, dim = "y", W
374+
text = self.axis_text_y_max_width(ax, side)
375+
ticks = self.axis_ticks_y_max_width(ax, side)
376+
facing = "r" if side == "left" else "l"
377+
band = text + ticks
378+
if text:
379+
m = theme.get_margin(f"axis_text_{g}_{side}").fig
380+
band += getattr(m, facing)
381+
if not band:
382+
return 0
383+
pad_pt = theme.getp(f"strip_switch_pad_{g}") or 0
384+
return band + (pad_pt / 72) / dim
385+
386+
def axis_ticks_x_max_height(self, ax: Axes, side: str) -> float:
354387
"""
355388
Return maximum height[figure space] of visible x ticks on a side
356389
"""
357390
attr = side_artists(side)[0]
358391
heights = [
359392
self.geometry.tight_height(getattr(tick, attr))
360-
for ax in self._filter_axes(location)
361393
for tick in self.axis_ticks_x(ax)
362394
if getattr(tick, attr).get_visible()
363395
]
364396
return max(heights) if len(heights) else 0
365397

398+
def axis_ticks_x_max_height_at(
399+
self, location: AxesLocation, side: str
400+
) -> float:
401+
"""
402+
Return maximum height[figure space] of visible x ticks on a side
403+
"""
404+
heights = [
405+
self.axis_ticks_x_max_height(ax, side)
406+
for ax in self._filter_axes(location)
407+
]
408+
return max(heights) if len(heights) else 0
409+
366410
def axis_text_x_max_height(self, ax: Axes, side: str) -> float:
367411
"""
368412
Return maximum height[figure space] of x tick labels on a side
@@ -385,21 +429,30 @@ def axis_text_x_max_height_at(
385429
]
386430
return max(heights) if len(heights) else 0
387431

388-
def axis_ticks_y_max_width_at(
389-
self, location: AxesLocation, side: str
390-
) -> float:
432+
def axis_ticks_y_max_width(self, ax: Axes, side: str) -> float:
391433
"""
392434
Return maximum width[figure space] of visible y ticks on a side
393435
"""
394436
attr = side_artists(side)[0]
395437
widths = [
396438
self.geometry.tight_width(getattr(tick, attr))
397-
for ax in self._filter_axes(location)
398439
for tick in self.axis_ticks_y(ax)
399440
if getattr(tick, attr).get_visible()
400441
]
401442
return max(widths) if len(widths) else 0
402443

444+
def axis_ticks_y_max_width_at(
445+
self, location: AxesLocation, side: str
446+
) -> float:
447+
"""
448+
Return maximum width[figure space] of visible y ticks on a side
449+
"""
450+
widths = [
451+
self.axis_ticks_y_max_width(ax, side)
452+
for ax in self._filter_axes(location)
453+
]
454+
return max(widths) if len(widths) else 0
455+
403456
def axis_text_y_max_width(self, ax: Axes, side: str) -> float:
404457
"""
405458
Return maximum width[figure space] of y tick labels on a side
@@ -552,7 +605,7 @@ def _move_artists(self, spaces: PlotSideSpaces):
552605
self._adjust_axis_text_x(justify, spaces)
553606
self._adjust_axis_text_y(justify, spaces)
554607
self._position_strip_band_axes(spaces)
555-
self._position_strip_backgrounds(spaces)
608+
self._position_strip_backgrounds()
556609

557610
def _adjust_axis_text_x(
558611
self, justify: TextJustifier, spaces: PlotSideSpaces
@@ -575,8 +628,8 @@ def to_vertical_axis_dimensions(value: float, ax: Axes) -> float:
575628
# For strip_placement="inside", an axis sharing its side with a
576629
# strip is pushed past the strip; zero otherwise.
577630
band_offsets = {
578-
"bottom": spaces.b.strip_band_offset("axis"),
579-
"top": spaces.t.strip_band_offset("axis"),
631+
"bottom": spaces.b.strip_band_offset(),
632+
"top": spaces.t.strip_band_offset(),
580633
}
581634

582635
for side in ("bottom", "top"):
@@ -647,8 +700,8 @@ def to_horizontal_axis_dimensions(value: float, ax: Axes) -> float:
647700
# For strip_placement="inside", an axis sharing its side with a
648701
# strip is pushed past the strip; zero otherwise.
649702
band_offsets = {
650-
"left": spaces.l.strip_band_offset("axis"),
651-
"right": spaces.r.strip_band_offset("axis"),
703+
"left": spaces.l.strip_band_offset(),
704+
"right": spaces.r.strip_band_offset(),
652705
}
653706

654707
for side in ("left", "right"):
@@ -690,10 +743,10 @@ def _position_strip_band_axes(self, spaces: PlotSideSpaces):
690743
to_points = 72 / fig.dpi
691744
W, H = fig.bbox.width, fig.bbox.height
692745
offsets = {
693-
"top": spaces.t.strip_band_offset("axis") * H,
694-
"bottom": spaces.b.strip_band_offset("axis") * H,
695-
"left": spaces.l.strip_band_offset("axis") * W,
696-
"right": spaces.r.strip_band_offset("axis") * W,
746+
"top": spaces.t.strip_band_offset() * H,
747+
"bottom": spaces.b.strip_band_offset() * H,
748+
"left": spaces.l.strip_band_offset() * W,
749+
"right": spaces.r.strip_band_offset() * W,
697750
}
698751
for ax in self.plot.axs:
699752
for side, offset in offsets.items():
@@ -718,33 +771,33 @@ def _strip_breadth_scales(
718771
largest = max(natural)
719772
return [largest / b for b in natural]
720773

721-
def _position_strip_backgrounds(self, spaces: PlotSideSpaces):
774+
def _position_strip_backgrounds(self):
722775
"""
723776
Fix each strip background at its final bounds and place its text
724777
725-
When `strip_placement="outside"` and a moved axis shares the
726-
strip's side, the strip is shifted outward to clear the axis.
778+
When `strip_placement="outside"`, each strip is shifted outward
779+
to clear the axis its own panel draws on the strip's side.
727780
"""
728781
theme = self.plot.theme
729782
groups = (
730-
(self.strip_text_x_top or [], spaces.t),
731-
(self.strip_text_x_bottom or [], spaces.b),
732-
(self.strip_text_y_right or [], spaces.r),
733-
(self.strip_text_y_left or [], spaces.l),
783+
self.strip_text_x_top or [],
784+
self.strip_text_x_bottom or [],
785+
self.strip_text_y_right or [],
786+
self.strip_text_y_left or [],
734787
)
735-
for group, space in groups:
788+
for group in groups:
736789
if not group:
737790
continue
738-
offset = space.strip_band_offset("strip")
739791
breadth = StripSpec.make(group[0], theme).breadth
740792
scales = self._strip_breadth_scales(group, breadth)
741793
for st, scale in zip(group, scales):
742794
spec = StripSpec.make(st, theme)
743795
x0, y0, w, h = self.strip_patch_bbox(st, scale).bounds
796+
shift = self.strip_shift(st)
744797
if spec.axis == "x":
745-
y0 += spec.sign * offset
798+
y0 += spec.sign * shift
746799
else:
747-
x0 += spec.sign * offset
800+
x0 += spec.sign * shift
748801
st.patch.set_bounds((x0, y0, w, h))
749802
self._position_strip_text(st)
750803

plotnine/_mpl/layout_manager/_plot_side_space.py

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -171,33 +171,30 @@ def _strip_switch_pad(self, axis: Literal["x", "y"]) -> float:
171171
dim = H if axis == "x" else W
172172
return (pad_pt / 72) / dim
173173

174-
def strip_band_offset(self, member: Literal["strip", "axis"]) -> float:
174+
def strip_band_offset(self) -> float:
175175
"""
176-
Outward offset for one member of a shared strip/axis band
176+
Outward offset of the axis within a shared strip/axis band
177177
178178
When a facet strip and an axis occupy the same side, the band is
179179
ordered from the panel outward. `strip_placement` chooses the order:
180180
181181
"inside": panel | strip | ticks+labels | title
182182
"outside": panel | ticks+labels | strip | title
183183
184-
`member` is `"strip"` (the strip background + text) or `"axis"` (the
185-
ticks and tick labels). The offset is produced here in figure space and
186-
consumed per artist in its own coordinate system: the title in figure
187-
space, the tick labels in axes fractions, the spine in points.
184+
For `"inside"` the axis (ticks and tick labels) clears the strip,
185+
so the offset is the strip breadth; for `"outside"` the axis stays
186+
at the panel and each strip clears its own panel's axis instead
187+
(`PlotLayoutItems.strip_shift`). The offset is produced here in
188+
figure space and consumed per artist in its own coordinate system:
189+
the tick labels in axes fractions, the spine in points.
188190
189191
The offset is zero unless the side carries both a strip and an axis.
190192
"""
191193
strip_breadth: float = self.strip_text # pyright: ignore[reportAttributeAccessIssue]
192-
axis_breadth = self._axis_ticks_and_text
193-
if not (strip_breadth and axis_breadth):
194+
if not (strip_breadth and self._axis_ticks_and_text):
194195
return 0
195196
placement = self.items.plot.theme.getp("strip_placement")
196-
if placement == "inside":
197-
return 0 if member == "strip" else strip_breadth
198-
# "outside": the strip clears the axis, plus the switch pad
199-
pad: float = self.strip_switch_pad # pyright: ignore[reportAttributeAccessIssue]
200-
return axis_breadth + pad if member == "strip" else 0
197+
return strip_breadth if placement == "inside" else 0
201198

202199

203200
class left_space(_plot_side_space):

tests/test_strip_placement.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,18 @@ def test_facet_wrap_top_outside():
3636
assert p1 == "facet_wrap_top_outside"
3737

3838

39+
def test_facet_wrap_top_outside_two_rows():
40+
# A panel that draws no axis on the strip's side keeps its strip
41+
# next to the panel; only axis-bearing panels shift theirs.
42+
p1 = (
43+
p
44+
+ facet_wrap("cyl", nrow=2)
45+
+ scale_x_continuous(position="top")
46+
+ theme(strip_placement="outside")
47+
)
48+
assert p1 == "facet_wrap_top_outside_two_rows"
49+
50+
3951
def test_facet_grid_top_inside():
4052
p1 = p + facet_grid("am", "cyl") + scale_x_continuous(position="top")
4153
assert p1 == "facet_grid_top_inside"

0 commit comments

Comments
 (0)