Deprecate style.copy(applicator); rework style/applicator setting mechanism

changes/224.bugfix.rst

@@ -0,0 +1 @@
+Fixed a bug that could cause nodes/widgets to attempt to apply their styles before their implementation is available.

src/travertino/declaration.py

@@ -326,8 +326,17 @@ def update(self, **styles):
     def copy(self, applicator=None):
         "Create a duplicate of this style declaration."
         dup = self.__class__()
-        dup._applicator = applicator
         dup.update(**self)
+
+        if applicator is not None:
+            warn(
+                "Providing an applicator to BaseStyle.copy() is deprecated. Set "
+                "applicator afterward on the returned copy.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+            dup._applicator = applicator
+
         return dup
 
     def __getitem__(self, name):

src/travertino/node.py

@@ -1,9 +1,13 @@
+from warnings import filterwarnings, warn
+
+# Make sure deprecation warnings are shown by default
+filterwarnings("default", category=DeprecationWarning)
+
+
 class Node:
     def __init__(self, style, applicator=None, children=None):
         self.applicator = applicator
-        self.style = style.copy(applicator)
-        self.intrinsic = self.style.IntrinsicSize()
-        self.layout = self.style.Box(self)
+        self.style = style
 
         self._parent = None
         self._root = None
@@ -14,6 +18,71 @@ def __init__(self, style, applicator=None, children=None):
             for child in children:
                 self.add(child)
 
+    @property
+    def style(self):
+        """The node's style.
+
+        Assigning a style triggers an application of that style if an applicator has
+        already been assigned.
+        """
+
+        return getattr(self, "_style", None)
+
+    @style.setter
+    def style(self, style):
+        if style is None:
+            self._style = None
+            return
+
+        self._style = style.copy()
+        self.intrinsic = self.style.IntrinsicSize()
+        self.layout = self.style.Box(self)
+
+        if self.applicator:
+            self.style._applicator = self.applicator
+
+            try:
+                self.style.reapply()
+            # Backwards compatibility for Toga
+            except AttributeError:
+                warn(
+                    "Failed to apply style, when new style assigned and applicator "
+                    "already present. Node should be sufficiently initialized to "
+                    "apply style before it has both a style and an applicator.",
+                    DeprecationWarning,
+                    stacklevel=2,
+                )
+
+    @property
+    def applicator(self):
+        """This node's applicator, which handles applying the style.
+
+        Assigning an applicator triggers an application of that style if a style has
+        already been assigned.
+        """
+        return getattr(self, "_applicator", None)
+
+    @applicator.setter
+    def applicator(self, applicator):
+        self._applicator = applicator
+
+        if self.style is not None:
+            self.style._applicator = applicator
+
+            if applicator:
+
+                try:
+                    self.style.reapply()
+                # Backwards compatibility for Toga
+                except AttributeError:
+                    warn(
+                        "Failed to apply style, when new applicator assigned and style "
+                        "already present. Node should be sufficiently initialized to "
+                        "apply style before it has both a style and an applicator.",
+                        DeprecationWarning,
+                        stacklevel=2,
+                    )
+
     @property
     def root(self):
         """The root of the tree containing this node.

tests/test_choices.py

@@ -1,38 +1,14 @@
 from __future__ import annotations
 
-import sys
-from dataclasses import dataclass
-from unittest.mock import Mock
 from warnings import catch_warnings, filterwarnings
 
 import pytest
 
+from tests.utils import mock_attr, prep_style_class
 from travertino.colors import NAMED_COLOR, rgb
 from travertino.constants import GOLDENROD, NONE, REBECCAPURPLE, TOP
 from travertino.declaration import BaseStyle, Choices, validated_property
 
-if sys.version_info < (3, 10):
-    _DATACLASS_KWARGS = {"init": False}
-else:
-    _DATACLASS_KWARGS = {"kw_only": True}
-
-
-def prep_style_class(cls):
-    """Decorator to apply dataclass and mock a style class's apply method."""
-    return mock_apply(dataclass(**_DATACLASS_KWARGS)(cls))
-
-
-def mock_apply(cls):
-    """Only mock apply, without applying dataclass."""
-    orig_init = cls.__init__
-
-    def __init__(self, *args, **kwargs):
-        self.apply = Mock()
-        orig_init(self, *args, **kwargs)
-
-    cls.__init__ = __init__
-    return cls
-
 
 @prep_style_class
 class Style(BaseStyle):
@@ -56,7 +32,7 @@ class Style(BaseStyle):
 with catch_warnings():
     filterwarnings("ignore", category=DeprecationWarning)
 
-    @mock_apply
+    @mock_attr("apply")
     class DeprecatedStyle(BaseStyle):
         pass
 

tests/test_declaration.py

@@ -5,7 +5,7 @@
 
 import pytest
 
-from tests.test_choices import mock_apply, prep_style_class
+from tests.utils import mock_attr, prep_style_class
 from travertino.declaration import (
     BaseStyle,
     Choices,
@@ -51,7 +51,7 @@ class Style(BaseStyle):
 with catch_warnings():
     filterwarnings("ignore", category=DeprecationWarning)
 
-    @mock_apply
+    @mock_attr("apply")
     class DeprecatedStyle(BaseStyle):
         pass
 
@@ -120,6 +120,13 @@ def test_create_and_copy(StyleClass):
     assert dup.implicit == VALUE3
 
 
+@pytest.mark.parametrize("StyleClass", [Style, DeprecatedStyle])
+def test_deprecated_copy(StyleClass):
+    style = StyleClass(explicit_const=VALUE2)
+    with pytest.warns(DeprecationWarning):
+        style.copy(applicator=object())
+
+
 @pytest.mark.parametrize("StyleClass", [Style, DeprecatedStyle])
 def test_reapply(StyleClass):
     style = StyleClass(explicit_const=VALUE2, implicit=VALUE3)

tests/test_node.py

@@ -1,12 +1,34 @@
 import pytest
 
-from travertino.declaration import BaseStyle
+from tests.utils import mock_attr, prep_style_class
+from travertino.declaration import BaseStyle, Choices, validated_property
 from travertino.layout import BaseBox, Viewport
 from travertino.node import Node
 from travertino.size import BaseIntrinsicSize
 
 
+@prep_style_class
+@mock_attr("reapply")
 class Style(BaseStyle):
+    int_prop: int = validated_property(Choices(integer=True))
+
+    class IntrinsicSize(BaseIntrinsicSize):
+        pass
+
+    class Box(BaseBox):
+        pass
+
+    def layout(self, root, viewport):
+        # A simple layout scheme that allocates twice the viewport size.
+        root.layout.content_width = viewport.width * 2
+        root.layout.content_height = viewport.height * 2
+
+
+@prep_style_class
+class BrokenStyle(BaseStyle):
+    def reapply(self):
+        raise AttributeError
+
     class IntrinsicSize(BaseIntrinsicSize):
         pass
 
@@ -219,3 +241,64 @@ def test_clear():
         assert child.root == child
 
     assert node.children == []
+
+
+def test_style_and_applicator_assignment():
+    style_1 = Style(int_prop=5)
+    node = Node(style=style_1)
+
+    # Style copies on assignment.
+    assert isinstance(node.style, Style)
+    assert node.style == style_1
+    assert node.style is not style_1
+
+    # Since no applicator has been assigned, style wasn't applied.
+    node.style.reapply.assert_not_called()
+
+    applicator = object()
+    node.applicator = applicator
+
+    # Applicator assignment does *not* copy.
+    assert node.applicator is applicator
+    # Should have also assigned to the style.
+    assert node.style._applicator is applicator
+    # Reapply should be called, now that we have both style and applicator.
+    node.style.reapply.assert_called_once()
+
+    style_2 = Style(int_prop=6)
+    node.style = style_2
+
+    assert node.style == style_2
+    # Since applicator is already assigned, assigning style triggers reapply.
+    node.style.reapply.assert_called_once()
+
+    node.style.reapply.reset_mock()
+
+    # Assigning None to applicator does not trigger reapply.
+    node.applicator = None
+    assert node.applicator is None
+    assert node.style._applicator is None
+    node.style.reapply.assert_not_called()
+
+    node.style = None
+    assert node.style is None
+
+    # There's no mock to test, but the fact that this doesn't raise an exception means
+    # it didn't try to call reapply.
+    node.applicator = object()
+
+
+def test_style_and_applicator_assignment_deprecated():
+    style = BrokenStyle()
+    applicator = object()
+
+    with pytest.warns(DeprecationWarning):
+        Node(style=style, applicator=applicator)
+
+    with pytest.warns(DeprecationWarning):
+        node = Node(style=style)
+        node.applicator = applicator
+
+    with pytest.warns(DeprecationWarning):
+        node = Node(style=None, applicator=applicator)
+        node.style = style

tests/utils.py

@@ -0,0 +1,29 @@
+import sys
+from dataclasses import dataclass
+from unittest.mock import Mock
+
+if sys.version_info < (3, 10):
+    _DATACLASS_KWARGS = {"init": False}
+else:
+    _DATACLASS_KWARGS = {"kw_only": True}
+
+
+def prep_style_class(cls):
+    """Decorator to apply dataclass and mock apply."""
+    return mock_attr("apply")(dataclass(**_DATACLASS_KWARGS)(cls))
+
+
+def mock_attr(attr):
+    """Mock an arbitrary attribute of a class."""
+
+    def returned_decorator(cls):
+        orig_init = cls.__init__
+
+        def __init__(self, *args, **kwargs):
+            setattr(self, attr, Mock())
+            orig_init(self, *args, **kwargs)
+
+        cls.__init__ = __init__
+        return cls
+
+    return returned_decorator