Skip to content

Commit c7b9867

Browse files
committed
Raise when converting a margin before setup()
Replace the silent ZeroDivisionError suppression in margin.to() with an explicit _is_setup flag. A margin must be attached to a themeable in a theme (which calls setup()) before any unit conversion; calling .to(...) earlier used to silently produce a margin with the new unit label but unchanged numeric values.
1 parent 6fb0180 commit c7b9867

2 files changed

Lines changed: 78 additions & 11 deletions

File tree

plotnine/themes/elements/margin.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@
44

55
from __future__ import annotations
66

7-
from contextlib import suppress
87
from copy import copy
98
from dataclasses import dataclass, field
109
from typing import TYPE_CHECKING
1110

11+
from ...exceptions import PlotnineError
12+
1213
if TYPE_CHECKING:
1314
from typing import Callable, Literal
1415

@@ -57,6 +58,10 @@ class margin:
5758
Size of the figure in inches
5859
"""
5960

61+
_is_setup: bool = field(
62+
init=False, default=False, repr=False, compare=False
63+
)
64+
6065
def setup(self, theme: theme, themeable_name: str):
6166
"""
6267
Setup the margin to be used in the layout
@@ -68,6 +73,7 @@ def setup(self, theme: theme, themeable_name: str):
6873
self.themeable_name = themeable_name
6974
self.fontsize = theme.getp((themeable_name, "size"), 11)
7075
self.figure_size = theme.getp("figure_size")
76+
self._is_setup = True
7177

7278
@property
7379
def pt(self) -> margin:
@@ -107,22 +113,23 @@ def to(self, unit: Literal["pt", "in", "lines", "fig"]) -> margin:
107113
"""
108114
Return margin in request unit
109115
"""
116+
if not self._is_setup:
117+
raise PlotnineError(
118+
"Cannot convert a margin that has not been set up. "
119+
"Call margin.setup() (or attach the margin to a "
120+
"themeable in a theme) first."
121+
)
122+
110123
m = copy(self)
111124
if self.unit == unit:
112125
return m
113126

114127
conversion = f"{self.unit}-{unit}"
115128
W, H = self.figure_size
116-
117-
with suppress(ZeroDivisionError):
118-
m.t = self._convert(conversion, H, self.t)
119-
with suppress(ZeroDivisionError):
120-
m.r = self._convert(conversion, W, self.r)
121-
with suppress(ZeroDivisionError):
122-
m.b = self._convert(conversion, H, self.b)
123-
with suppress(ZeroDivisionError):
124-
m.l = self._convert(conversion, W, self.l)
125-
129+
m.t = self._convert(conversion, H, self.t)
130+
m.r = self._convert(conversion, W, self.r)
131+
m.b = self._convert(conversion, H, self.b)
132+
m.l = self._convert(conversion, W, self.l)
126133
m.unit = unit
127134
return m
128135

tests/test_margin.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import pytest
2+
3+
from plotnine import element_text, theme
4+
from plotnine.exceptions import PlotnineError
5+
from plotnine.themes.elements.margin import margin
6+
7+
8+
def setup_margin(
9+
m: margin, fontsize: float = 11, figure_size=(8.0, 6.0)
10+
) -> margin:
11+
"""Attach margin to a theme and call its setup() as the layout would"""
12+
t = theme(
13+
figure_size=figure_size,
14+
plot_title=element_text(size=fontsize, margin=m),
15+
)
16+
m.setup(t, "plot_title")
17+
return m
18+
19+
20+
def test_conversion_before_setup_raises():
21+
m = margin(t=5, unit="pt")
22+
with pytest.raises(PlotnineError, match="set up"):
23+
m.to("fig")
24+
25+
26+
def test_conversion_same_unit_after_setup_succeeds():
27+
m = setup_margin(margin(t=5, r=5, b=5, l=5, unit="pt"))
28+
result = m.to("pt")
29+
assert result.unit == "pt"
30+
assert result.t == 5
31+
32+
33+
def test_lines_to_pt_after_setup():
34+
m = setup_margin(margin(t=3, r=3, b=3, l=3, unit="lines"))
35+
result = m.to("pt")
36+
assert result.unit == "pt"
37+
assert result.t == pytest.approx(33)
38+
assert result.r == pytest.approx(33)
39+
assert result.b == pytest.approx(33)
40+
assert result.l == pytest.approx(33)
41+
42+
43+
def test_conversion_fig_isotropy_after_setup():
44+
"""Physical distances match on all sides for equal lines input"""
45+
W, H = 12.0, 4.0
46+
m = setup_margin(
47+
margin(t=3, r=3, b=3, l=3, unit="lines"), figure_size=(W, H)
48+
)
49+
mf = m.to("fig")
50+
assert mf.t * H == pytest.approx(mf.l * W)
51+
assert mf.b * H == pytest.approx(mf.r * W)
52+
53+
54+
def test_conversion_with_zero_figure_dimension_raises():
55+
"""Zero figure dimension propagates ZeroDivisionError on conversion"""
56+
m = setup_margin(
57+
margin(t=3, r=3, b=3, l=3, unit="lines"), figure_size=(0.0, 6.0)
58+
)
59+
with pytest.raises(ZeroDivisionError):
60+
m.to("fig")

0 commit comments

Comments
 (0)