Fix regressions in tunables/injections

- ClassVar annotations were treated as injectable dependencies
- Toposort setup dependency graph included non-component injectables

Author: virtuald

magicbot/inject.py

@@ -1,4 +1,6 @@
+import inspect
 import logging
+import typing
 from typing import Any, Optional
 
 logger = logging.getLogger(__name__)
@@ -28,12 +30,22 @@ def get_injection_requests(
                 raise MagicInjectError(message)
             continue
 
-        # If the variable has been set, skip it
-        if component is not None and hasattr(component, n):
+        # If the variable has been set (on instance or class), skip it.
+        # Use getattr_static so descriptors (ex: tunable) do not execute.
+        if component is not None:
+            try:
+                inspect.getattr_static(component, n)
+            except AttributeError:
+                pass
+            else:
+                continue
+
+        # Ignore class variables, they are not injection requests
+        origin = getattr(inject_type, "__origin__", None)
+        if origin is typing.ClassVar:
             continue
 
         # Check for generic types from the typing module
-        origin = getattr(inject_type, "__origin__", None)
         if origin is not None:
             inject_type = origin
 

magicbot/magicrobot.py

@@ -624,6 +624,8 @@ def _create_components(self) -> None:
 
         # Sort components so that setup functions can rely on the setup of an
         # injected variable being called already
+        component_by_id = {id(component): cname for cname, component in components}
+
         dag = {}
         for cname, component in components:
             setup = getattr(component, "setup", None)
@@ -632,7 +634,18 @@ def _create_components(self) -> None:
             else:
                 type_hints = typing.get_type_hints(type(component))
                 requests = get_injection_requests(type_hints, cname, component)
-                dag[cname] = list(requests.keys())
+
+                deps = []
+                for n in requests:
+                    injectable = injectables.get(n)
+                    if injectable is None:
+                        injectable = injectables.get(f"{cname}_{n}")
+
+                    dep_name = component_by_id.get(id(injectable))
+                    if dep_name is not None:
+                        deps.append(dep_name)
+
+                dag[cname] = deps
 
         # Create an ordered component list based on possible setup() dependencies
         setup_ordered_components = [

tests/test_magicbot_inject.py

@@ -1,8 +1,10 @@
 import typing
+from typing import ClassVar
 
 import pytest
 
 from magicbot.inject import get_injection_requests
+from magicbot.magic_tunable import tunable
 
 
 def test_ctor_invalid_type_hint_message():
@@ -18,3 +20,27 @@ def __init__(self, foo: 1): ...
         get_injection_requests(type_hints, "bar")
 
     assert exc_info.value.args[0] == "Component bar has a non-type annotation foo: 1"
+
+
+def test_component_classvar_annotation_is_not_treated_as_injection_request():
+    class Component:
+        injected: int
+        state_names: ClassVar[int]
+
+    requests = get_injection_requests(
+        typing.get_type_hints(Component), "component", Component()
+    )
+
+    assert requests == {"injected": int}
+
+
+def test_component_tunable_annotation_is_not_treated_as_injection_request():
+    class Component:
+        injected: int
+        speed: tunable[float] = tunable(0.0)
+
+    requests = get_injection_requests(
+        typing.get_type_hints(Component), "component", Component()
+    )
+
+    assert requests == {"injected": int}

tests/test_magicbot_injection.py

@@ -199,6 +199,20 @@ def createObjects(self) -> None:
         pass
 
 
+class ExternalDependencyComponent(DumbComponent):
+    setting: float
+
+    def setup(self):
+        pass
+
+
+class ExternalDependencyBot(magicbot.MagicRobot):
+    component: ExternalDependencyComponent
+
+    def createObjects(self) -> None:
+        self.component_setting = 2.5
+
+
 R = TypeVar("R", bound=magicbot.MagicRobot)
 
 
@@ -282,3 +296,9 @@ def test_toposort_inject():
     expected_names = ["a", "b", "c", "d"]
     for (name, _), expected in zip(bot._components, expected_names):
         assert name == expected
+
+
+def test_toposort_ignores_non_component_dependencies():
+    bot = _make_bot(ExternalDependencyBot)
+
+    assert bot.component.setting == 2.5