From f4c577a5c3fe969d9848427acda338658ee6a12b Mon Sep 17 00:00:00 2001
From: Hassan Kibirige <has2k1@gmail.com>
Date: Wed, 3 Apr 2024 14:28:36 +0300
Subject: [PATCH] Fix setting text artists to blank

fixes #764
---
 doc/changelog.qmd                        |   8 ++
 plotnine/ggplot.py                       |   6 +-
 plotnine/guides/guide.py                 |  16 +--
 plotnine/guides/guide_colorbar.py        |   2 +-
 plotnine/guides/guide_legend.py          |   3 -
 plotnine/guides/guides.py                | 148 +++++++++++------------
 plotnine/themes/elements/element_text.py |   4 +-
 plotnine/themes/targets.py               |   6 +-
 plotnine/themes/theme.py                 |  86 ++++++-------
 plotnine/themes/theme_gray.py            |   1 +
 plotnine/themes/theme_matplotlib.py      |   1 +
 plotnine/themes/theme_seaborn.py         |   2 +-
 plotnine/themes/themeable.py             |   9 ++
 13 files changed, 142 insertions(+), 150 deletions(-)

diff --git a/doc/changelog.qmd b/doc/changelog.qmd
index ce5984ab3..fee9e9bd2 100644
--- a/doc/changelog.qmd
+++ b/doc/changelog.qmd
@@ -2,6 +2,14 @@
 title: Changelog
 ---
 
+## v0.13.4
+(not-yet-released)
+
+### Bug Fixes
+
+- Fixed regression in v0.13.3 where setting some text elements `element_blank` led to an
+  error. {{< issue 764 >}}
+
 ## v0.13.3
 (2024-03-27)
 [![](https://zenodo.org/badge/DOI/10.5281/zenodo.10887259.svg)](https://doi.org/10.5281/zenodo.10887259)
diff --git a/plotnine/ggplot.py b/plotnine/ggplot.py
index 85f94073d..822024c5a 100755
--- a/plotnine/ggplot.py
+++ b/plotnine/ggplot.py
@@ -273,13 +273,14 @@ def draw(self, show: bool = False) -> Figure:
 
             # setup
             self.figure, self.axs = self.facet.setup(self)
+            self.guides._setup(self)
             self.theme.setup(self)
 
             # Drawing
             self._draw_layers()
             self._draw_panel_borders()
             self._draw_breaks_and_labels()
-            self.guides.draw(self)
+            self.guides.draw()
             self._draw_figure_texts()
             self._draw_watermarks()
 
@@ -314,12 +315,13 @@ def _draw_using_figure(self, figure: Figure, axs: list[Axes]) -> ggplot:
 
             # setup
             self.figure, self.axs = self.facet.setup(self)
+            self.guides._setup(self)
             self.theme.setup(self)
 
             # drawing
             self._draw_layers()
             self._draw_breaks_and_labels()
-            self.guides.draw(self)
+            self.guides.draw()
 
             # artist theming
             self.theme.apply()
diff --git a/plotnine/guides/guide.py b/plotnine/guides/guide.py
index 168745955..e80f70b7e 100644
--- a/plotnine/guides/guide.py
+++ b/plotnine/guides/guide.py
@@ -114,6 +114,7 @@ def setup(self, guides: guides):
         # guide theme has priority and its targets are tracked
         # independently.
         self.theme = guides.plot.theme + self.theme
+        self.theme.setup(guides.plot)
         self.plot_layers = guides.plot.layers
         self.plot_mapping = guides.plot.mapping
         self.elements = self._elements_cls(self.theme, self)
@@ -189,11 +190,8 @@ def title(self):
         ha = self.theme.getp(("legend_title", "ha"))
         va = self.theme.getp(("legend_title", "va"), "center")
         _margin = self.theme.getp(("legend_title", "margin"))
-        if _margin is not None:
-            _loc = get_opposite_side(self.title_position)[0]
-            margin = _margin.get_as(_loc, "pt")
-        else:
-            margin = 0
+        _loc = get_opposite_side(self.title_position)[0]
+        margin = _margin.get_as(_loc, "pt")
         top_or_bottom = self.title_position in ("top", "bottom")
         is_blank = self.theme.T.is_blank("legend_title")
 
@@ -219,12 +217,8 @@ def text_position(self) -> SidePosition:
     @cached_property
     def _text_margin(self) -> float:
         _margin = self.theme.getp((f"legend_text_{self.guide_kind}", "margin"))
-        if _margin is not None:
-            _loc = get_opposite_side(self.text_position)
-            margin = _margin.get_as(_loc[0], "pt")
-        else:
-            margin = 0
-        return margin
+        _loc = get_opposite_side(self.text_position)
+        return _margin.get_as(_loc[0], "pt")
 
     @cached_property
     def title_position(self) -> SidePosition:
diff --git a/plotnine/guides/guide_colorbar.py b/plotnine/guides/guide_colorbar.py
index f9f6e768c..6b0fa834d 100644
--- a/plotnine/guides/guide_colorbar.py
+++ b/plotnine/guides/guide_colorbar.py
@@ -411,7 +411,7 @@ def add_labels(
     labels: Sequence[str],
     ys: Sequence[float],
     elements: GuideElementsColorbar,
-) -> Sequence[Text]:
+) -> list[Text]:
     """
     Return Texts added to the auxbox
     """
diff --git a/plotnine/guides/guide_legend.py b/plotnine/guides/guide_legend.py
index f5d43185f..995097b8d 100644
--- a/plotnine/guides/guide_legend.py
+++ b/plotnine/guides/guide_legend.py
@@ -23,7 +23,6 @@
     from matplotlib.artist import Artist
     from matplotlib.offsetbox import PackerBase
 
-    from plotnine import theme
     from plotnine.geoms.geom import geom
     from plotnine.layer import layer
     from plotnine.typing import SidePosition, TupleFloat2, TupleInt2
@@ -236,8 +235,6 @@ def draw(self):
 
         from .._mpl.offsetbox import ColoredDrawingArea
 
-        self.theme = cast("theme", self.theme)
-
         obverse = slice(0, None)
         reverse = slice(None, None, -1)
         nbreak = len(self.key)
diff --git a/plotnine/guides/guides.py b/plotnine/guides/guides.py
index aca6f3bcc..1e4817368 100644
--- a/plotnine/guides/guides.py
+++ b/plotnine/guides/guides.py
@@ -28,6 +28,7 @@
 
     from plotnine import ggplot, theme
     from plotnine.iapi import labels_view
+    from plotnine.scales.scale import scale
     from plotnine.scales.scales import Scales
     from plotnine.typing import (
         LegendOnly,
@@ -35,6 +36,7 @@
         LegendPosition,
         NoGuide,
         Orientation,
+        ScaledAestheticsName,
         SidePosition,
         TextJustification,
         TupleFloat2,
@@ -93,9 +95,10 @@ def __post_init__(self):
         self.plot_scales: Scales
         self.plot_labels: labels_view
         self.elements: GuidesElements
-
+        self._lookup: dict[tuple[scale, ScaledAestheticsName], guide] = {}
         if self.colour is not None and self.color is not None:
             raise ValueError("Got a guide for color and colour, choose one.")
+        rename_aesthetics(self)
 
     def __radd__(self, plot: ggplot):
         """
@@ -117,16 +120,6 @@ def __radd__(self, plot: ggplot):
 
         return plot
 
-    def _guide_lookup(self) -> dict[str, guide | NoGuide]:
-        """
-        Lookup dict for guides that have been set
-        """
-        return {
-            f.name: g
-            for f in fields(self)
-            if (g := getattr(self, f.name)) is not None
-        }
-
     def _build(self) -> Sequence[guide]:
         """
         Build the guides
@@ -139,18 +132,18 @@ def _build(self) -> Sequence[guide]:
         """
         return self._create_geoms(self._merge(self._train()))
 
-    def _train(self) -> Sequence[guide]:
+    def _setup(self, plot: ggplot):
         """
-        Compute all the required guides
-
-        Returns
-        -------
-        gdefs : list
-            Guides for the plots
+        Setup all guides that will be active
         """
-        gdefs: list[guide] = []
-        rename_aesthetics(self)
-        guide_lookup = self._guide_lookup()
+        self.plot = plot
+        self.elements = GuidesElements(self.plot.theme)
+
+        guide_lookup = {
+            f.name: g
+            for f in fields(self)
+            if (g := getattr(self, f.name)) is not None
+        }
 
         for scale in self.plot.scales:
             for ae in scale.aesthetics:
@@ -171,58 +164,62 @@ def _train(self) -> Sequence[guide]:
 
                 # check the validity of guide.
                 # if guide is str, then find the guide object
-                g = self._setup(g)
-
-                # Guide turned off
-                if not g.elements.position:
-                    continue
+                if isinstance(g, str):
+                    g = Registry[f"guide_{g}"]()
+                elif not isinstance(g, guide):
+                    raise PlotnineError(f"Unknown guide: {g}")
 
-                # check the consistency of the guide and scale.
-                if (
-                    "any" not in g.available_aes
-                    and scale.aesthetics[0] not in g.available_aes
-                ):
-                    raise PlotnineError(
-                        f"{g.__class__.__name__} cannot be used for "
-                        f"{scale.aesthetics}"
-                    )
-
-                # title
-                if g.title is None:
-                    if scale.name:
-                        g.title = scale.name
-                    else:
-                        g.title = getattr(self.plot.labels, ae)
-                        if g.title is None:
-                            warn(
-                                f"Cannot generate legend for the {ae!r} "
-                                "aesthetic. Make sure you have mapped a "
-                                "variable to it",
-                                PlotnineWarning,
-                            )
-
-                # each guide object trains scale within the object,
-                # so Guides (i.e., the container of guides)
-                # need not to know about them
-                g = g.train(scale, ae)
-
-                if g is not None:
-                    gdefs.append(g)
-
-        return gdefs
+                g.setup(self)
+                self._lookup[(scale, ae)] = g
 
-    def _setup(self, g: str | guide) -> guide:
-        """
-        Validate guide object
+    def _train(self) -> Sequence[guide]:
         """
-        if isinstance(g, str):
-            g = Registry[f"guide_{g}"]()
+        Compute all the required guides
 
-        if not isinstance(g, guide):
-            raise PlotnineError(f"Unknown guide: {g}")
+        Returns
+        -------
+        gdefs : list
+            Guides for the plots
+        """
+        gdefs: list[guide] = []
+        for (scale, ae), g in self._lookup.items():
+            # Guide turned off
+            if not g.elements.position:
+                continue
+
+            # check the consistency of the guide and scale.
+            if (
+                "any" not in g.available_aes
+                and scale.aesthetics[0] not in g.available_aes
+            ):
+                raise PlotnineError(
+                    f"{g.__class__.__name__} cannot be used for "
+                    f"{scale.aesthetics}"
+                )
+
+            # title
+            if g.title is None:
+                if scale.name:
+                    g.title = scale.name
+                else:
+                    g.title = getattr(self.plot.labels, ae)
+                    if g.title is None:
+                        warn(
+                            f"Cannot generate legend for the {ae!r} "
+                            "aesthetic. Make sure you have mapped a "
+                            "variable to it",
+                            PlotnineWarning,
+                        )
+
+            # each guide object trains scale within the object,
+            # so Guides (i.e., the container of guides)
+            # need not to know about them
+            g = g.train(scale, ae)
+
+            if g is not None:
+                gdefs.append(g)
 
-        g.setup(self)
-        return g
+        return gdefs
 
     def _merge(self, gdefs: Sequence[guide]) -> Sequence[guide]:
         """
@@ -274,7 +271,6 @@ def _apply_guide_themes(self, gdefs: list[guide]):
         Apply the theme for each guide
         """
         for g in gdefs:
-            g.theme = cast("theme", g.theme)
             g.theme.apply()
 
     def _assemble_guides(
@@ -347,24 +343,16 @@ def _anchored_offset_box(boxes: list[PackerBase]):
 
         return legends
 
-    def draw(self, plot: ggplot) -> Optional[OffsetBox]:
+    def draw(self) -> Optional[OffsetBox]:
         """
         Draw guides onto the figure
 
-        Parameters
-        ----------
-        plot :
-            ggplot object
-
         Returns
         -------
         :matplotlib.offsetbox.Offsetbox | None
             A box that contains all the guides for the plot.
             If there are no guides, **None** is returned.
         """
-        self.plot = plot
-        self.elements = GuidesElements(plot.theme)
-
         if self.elements.position == "none":
             return
 
@@ -387,9 +375,9 @@ def draw(self, plot: ggplot) -> Optional[OffsetBox]:
         self._apply_guide_themes(gdefs)
         legends = self._assemble_guides(gdefs, guide_boxes)
         for aob in legends.boxes:
-            plot.figure.add_artist(aob)
+            self.plot.figure.add_artist(aob)
 
-        plot.theme.targets.legends = legends
+        self.plot.theme.targets.legends = legends
 
 
 VALID_JUSTIFICATION_WORDS = {"left", "right", "top", "bottom", "center"}
diff --git a/plotnine/themes/elements/element_text.py b/plotnine/themes/elements/element_text.py
index 38bcb21b4..e1ae5e7e9 100644
--- a/plotnine/themes/elements/element_text.py
+++ b/plotnine/themes/elements/element_text.py
@@ -122,9 +122,8 @@ def __init__(
 
         super().__init__()
         self.properties.update(**kwargs)
-
         if margin is not None:
-            margin = Margin(self, **margin)  # type: ignore
+            self.properties["margin"] = Margin(self, **margin)
 
         # Use the parameters that have been set
         names = (
@@ -138,7 +137,6 @@ def __init__(
             "style",
             "va",
             "weight",
-            "margin",
         )
         variables = locals()
         for name in names:
diff --git a/plotnine/themes/targets.py b/plotnine/themes/targets.py
index 49b951da8..ce81ac10d 100644
--- a/plotnine/themes/targets.py
+++ b/plotnine/themes/targets.py
@@ -4,7 +4,7 @@
 from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
-    from typing import Optional, Sequence
+    from typing import Optional
 
     from matplotlib.collections import LineCollection
     from matplotlib.patches import Rectangle
@@ -30,8 +30,8 @@ class ThemeTargets:
     legend_frame: Optional[Rectangle] = None
     legend_key: list[ColoredDrawingArea] = field(default_factory=list)
     legends: Optional[legend_artists] = None
-    legend_text_colorbar: Sequence[Text] = field(default_factory=list)
-    legend_text_legend: Sequence[Text] = field(default_factory=list)
+    legend_text_colorbar: list[Text] = field(default_factory=list)
+    legend_text_legend: list[Text] = field(default_factory=list)
     legend_ticks: Optional[LineCollection] = None
     legend_title: Optional[Text] = None
     panel_border: list[Rectangle] = field(default_factory=list)
diff --git a/plotnine/themes/theme.py b/plotnine/themes/theme.py
index 56c476e7f..19ad6b844 100644
--- a/plotnine/themes/theme.py
+++ b/plotnine/themes/theme.py
@@ -222,7 +222,6 @@ def __init__(
     ):
         self.themeables = Themeables()
         self.complete = complete
-        self._is_setup = False
 
         if complete:
             self._rcParams = deepcopy(default_rcparams)
@@ -281,8 +280,6 @@ def apply(self):
         Subclasses that override this method should make sure that the
         base class method is called.
         """
-        self._add_default_themeable_properties()
-
         for th in self.T.values():
             th.apply(self)
 
@@ -297,18 +294,12 @@ def setup(self, plot: ggplot):
 
         It also initialises where the artists to be themed will be stored.
         """
-        from .elements.element_base import element_base
-
         self.plot = plot
         self.figure = plot.figure
         self.axs = plot.axs
         self.targets = ThemeTargets()
-
-        for name, th in self.T.items():
-            if isinstance(th.theme_element, element_base):
-                th.theme_element.setup(self, name)
-
-        self._is_setup = True
+        self._add_default_themeable_properties()
+        self.T.setup(self)
 
     def _add_default_themeable_properties(self):
         """
@@ -320,7 +311,7 @@ def _add_default_themeable_properties(self):
 
         This is where the theme is modified to add those values.
         """
-        smart_title_and_subtitle_ha(self)
+        self._smart_title_and_subtitle_ha()
 
     @property
     def rcParams(self):
@@ -366,9 +357,6 @@ def add_theme(self, other: theme) -> theme:
         A complete theme will annihilate any previous themes. Partial themes
         can be added together and can be added to a complete theme.
         """
-        if self._is_setup:
-            other.setup(self.plot)
-
         if other.complete:
             return other
 
@@ -445,8 +433,11 @@ def __deepcopy__(self, memo: dict) -> theme:
         new = result.__dict__
 
         shallow = {"plot", "figure", "axs"}
+        skip = {"targets"}
         for key, item in old.items():
-            if key in shallow:
+            if key in skip:
+                continue
+            elif key in shallow:
                 new[key] = item
                 memo[id(new[key])] = new[key]
             else:
@@ -463,6 +454,39 @@ def to_retina(self) -> theme:
         dpi = self.getp("dpi")
         return self + theme(dpi=dpi * 2)
 
+    def _smart_title_and_subtitle_ha(self):
+        """
+        Smartly add the horizontal alignment for the title and subtitle
+        """
+        from .elements import element_text
+
+        has_title = bool(
+            self.plot.labels.get("title", "")
+        ) and not self.T.is_blank("plot_title")
+        has_subtitle = bool(
+            self.plot.labels.get("subtitle", "")
+        ) and not self.T.is_blank("plot_subtitle")
+
+        title_ha = self.getp(("plot_title", "ha"))
+        subtitle_ha = self.getp(("plot_subtitle", "ha"))
+
+        default_title_ha, default_subtitle_ha = "center", "left"
+        kwargs = {}
+
+        if has_title and title_ha is None:
+            if has_subtitle and not subtitle_ha:
+                title_ha = default_subtitle_ha
+            else:
+                title_ha = default_title_ha
+            kwargs["plot_title"] = element_text(ha=title_ha)
+
+        if has_subtitle and subtitle_ha is None:
+            subtitle_ha = default_subtitle_ha
+            kwargs["plot_subtitle"] = element_text(ha=subtitle_ha)
+
+        if kwargs:
+            self += theme(**kwargs)
+
 
 def theme_get() -> theme:
     """
@@ -513,33 +537,3 @@ def theme_update(**kwargs: themeable):
     """
     assert "complete" not in kwargs
     theme_set(theme_get() + theme(**kwargs))  # pyright: ignore
-
-
-def smart_title_and_subtitle_ha(plot_theme: theme):
-    """
-    Smartly add the horizontal alignment for the title and subtitle
-    """
-    from .elements import element_text
-
-    has_title = plot_theme.targets.plot_title is not None
-    has_subtitle = plot_theme.targets.plot_subtitle is not None
-
-    title_ha = plot_theme.getp(("plot_title", "ha"))
-    subtitle_ha = plot_theme.getp(("plot_subtitle", "ha"))
-
-    default_title_ha, default_subtitle_ha = "center", "left"
-    kwargs = {}
-
-    if has_title and title_ha is None:
-        if has_subtitle and not subtitle_ha:
-            title_ha = default_subtitle_ha
-        else:
-            title_ha = default_title_ha
-        kwargs["plot_title"] = element_text(ha=title_ha)
-
-    if has_subtitle and subtitle_ha is None:
-        subtitle_ha = default_subtitle_ha
-        kwargs["plot_subtitle"] = element_text(ha=subtitle_ha)
-
-    if kwargs:
-        plot_theme += theme(**kwargs)
diff --git a/plotnine/themes/theme_gray.py b/plotnine/themes/theme_gray.py
index 8e38c5bbc..87720a52e 100644
--- a/plotnine/themes/theme_gray.py
+++ b/plotnine/themes/theme_gray.py
@@ -40,6 +40,7 @@ def __init__(self, base_size=11, base_family=None):
                 size=base_size,
                 linespacing=0.9,
                 rotation=0,
+                margin={},
             ),
             aspect_ratio=get_option("aspect_ratio"),
             axis_line=element_line(),
diff --git a/plotnine/themes/theme_matplotlib.py b/plotnine/themes/theme_matplotlib.py
index dc52e584f..476c89592 100644
--- a/plotnine/themes/theme_matplotlib.py
+++ b/plotnine/themes/theme_matplotlib.py
@@ -35,6 +35,7 @@ def __init__(self, rc=None, fname=None, use_defaults=True):
                 size=base_size,
                 linespacing=1,
                 rotation=0,
+                margin={},
             ),
             aspect_ratio=get_option("aspect_ratio"),
             axis_text=element_text(margin={"t": 2.4, "r": 2.4, "units": "pt"}),
diff --git a/plotnine/themes/theme_seaborn.py b/plotnine/themes/theme_seaborn.py
index 39501551a..ef2dff00a 100644
--- a/plotnine/themes/theme_seaborn.py
+++ b/plotnine/themes/theme_seaborn.py
@@ -46,7 +46,7 @@ def __init__(
             aspect_ratio=get_option("aspect_ratio"),
             dpi=get_option("dpi"),
             figure_size=get_option("figure_size"),
-            text=element_text(size=base_size, rotation=0),
+            text=element_text(size=base_size, rotation=0, margin={}),
             axis_text=element_text(
                 size=base_size * 0.8,
                 margin={
diff --git a/plotnine/themes/themeable.py b/plotnine/themes/themeable.py
index 24e2a7d53..ba52918af 100644
--- a/plotnine/themes/themeable.py
+++ b/plotnine/themes/themeable.py
@@ -314,6 +314,15 @@ def _dict(self):
                     result[name] = self[name]
         return result
 
+    def setup(self, theme: theme):
+        """
+        Setup themeables for theming
+        """
+        # Setup theme elements
+        for name, th in self.items():
+            if isinstance(th.theme_element, element_base):
+                th.theme_element.setup(theme, name)
+
     def items(self):
         """
         List of (name, themeable) in reverse based on the inheritance.