Core: Add decless traits (#69 & #72)

Additionally, simplify the trait impl implementation and extend trait testing

* Core: Add `reference` (#69)

* Core: Add megic_pointer to generate convenient references

* Library: Move Reference to library and add some polish

* Library: Add has_reference trait

* Get the references passing pre-commit

* Test: refactor test_basic.py to work as a pytest properly

* Library: Move reference back to core. Apparently library doesn't like it

* Test: Refactor test_basic with clear criteria

* Improved basic test; use type_pair

* graphinterface connect tying

* Address incidious underlying properties

* Add check to ensure properties aren't instantiated as instance attributes during node construction

* Reinstate better errors on field construction

* Add util to check debugging

* pre-commit

---------

Co-authored-by: iopapamanoglou <[email protected]>

src/faebryk/core/graphinterface.py

@@ -7,9 +7,17 @@
 
 from faebryk.core.core import ID_REPR, FaebrykLibObject
 from faebryk.core.graph_backends.default import GraphImpl
-from faebryk.core.link import Link, LinkDirect, LinkNamedParent
+from faebryk.core.link import (
+    Link,
+    LinkDirect,
+    LinkNamedParent,
+    LinkPointer,
+    LinkSibling,
+)
 from faebryk.libs.util import (
+    KeyErrorNotFound,
     exceptions_to_log,
+    find,
     not_none,
     try_avoid_endless_recursion,
 )
@@ -106,7 +114,7 @@ def is_connected(self, other: "GraphInterface"):
     # Less graph-specific stuff
 
     # TODO make link trait to initialize from list
-    def connect(self, other: Self, linkcls=None) -> Self:
+    def connect(self, other: "GraphInterface", linkcls=None) -> Self:
         assert other is not self
 
         if linkcls is None:
@@ -191,3 +199,22 @@ def disconnect_parent(self):
 
 
 class GraphInterfaceSelf(GraphInterface): ...
+
+
+class GraphInterfaceReference[T: "Node"](GraphInterface):
+    """Represents a reference to a node object"""
+
+    class UnboundError(Exception):
+        """Cannot resolve unbound reference"""
+
+    def get_referenced_gif(self) -> GraphInterfaceSelf:
+        try:
+            return find(
+                self.get_links_by_type(LinkPointer),
+                lambda link: not isinstance(link, LinkSibling),
+            ).pointee
+        except KeyErrorNotFound as ex:
+            raise GraphInterfaceReference.UnboundError from ex
+
+    def get_reference(self) -> T:
+        return self.get_referenced_gif().node

src/faebryk/core/link.py

@@ -5,11 +5,16 @@
 from typing import TYPE_CHECKING, Callable
 
 from faebryk.core.core import LINK_TB, FaebrykLibObject
+from faebryk.libs.util import is_type_pair
 
 logger = logging.getLogger(__name__)
 
 if TYPE_CHECKING:
-    from faebryk.core.graphinterface import GraphInterface, GraphInterfaceHierarchical
+    from faebryk.core.graphinterface import (
+        GraphInterface,
+        GraphInterfaceHierarchical,
+        GraphInterfaceSelf,
+    )
 
 
 class Link(FaebrykLibObject):
@@ -38,13 +43,31 @@ def __repr__(self) -> str:
         return f"{type(self).__name__}()"
 
 
-class LinkSibling(Link):
-    def __init__(self, interfaces: list["GraphInterface"]) -> None:
+class LinkPointer(Link):
+    """A Link that points towards a self-gif"""
+
+    def __init__(
+        self,
+        interfaces: list["GraphInterfaceSelf | GraphInterface"],
+    ) -> None:
+        from faebryk.core.graphinterface import GraphInterface, GraphInterfaceSelf
+
         super().__init__()
-        self.interfaces = interfaces
+        assert len(interfaces) == 2
+
+        pair = is_type_pair(
+            interfaces[0], interfaces[1], GraphInterfaceSelf, GraphInterface
+        )
+        if not pair:
+            raise TypeError("Interfaces must be one self-gif and one other-gif")
+        self.pointee, self.pointer = pair
 
     def get_connections(self) -> list["GraphInterface"]:
-        return self.interfaces
+        return [self.pointee, self.pointer]
+
+
+class LinkSibling(LinkPointer):
+    """A link represents a connection between a self-gif and a gif in the same node"""
 
 
 class LinkParent(Link):
@@ -83,9 +106,13 @@ def curried(interfaces: list["GraphInterface"]):
 
 
 class LinkDirect(Link):
+    """Represents a symmetrical link between two interfaces of the same type"""
+
     def __init__(self, interfaces: list["GraphInterface"]) -> None:
         super().__init__()
-        assert len(set(map(type, interfaces))) == 1
+        assert (
+            len(set(map(type, interfaces))) == 1
+        ), "Interfaces must be of the same type"
         self.interfaces = interfaces
 
     def get_connections(self) -> list["GraphInterface"]:

src/faebryk/core/node.py

@@ -1,6 +1,8 @@
 # This file is part of the faebryk project
 # SPDX-License-Identifier: MIT
 import logging
+from abc import abstractmethod
+from dataclasses import InitVar as dataclass_InitVar
 from itertools import chain
 from typing import (
     TYPE_CHECKING,
@@ -29,6 +31,7 @@
     PostInitCaller,
     Tree,
     cast_assert,
+    debugging,
     find,
     not_none,
     times,
@@ -65,13 +68,34 @@ class fab_field:
     pass
 
 
-class rt_field[T, O](property, fab_field):
+class constructed_field[T: "Node", O: "Node"](property, fab_field):
+    """
+    Field which is constructed after the node is created.
+    The constructor gets one argument: the node instance.
+
+    The constructor should return the constructed faebryk object or None.
+    If a faebryk object is returned, it will be added to the node.
+    """
+
+    @abstractmethod
+    def __construct__(self, obj: T) -> O | None:
+        pass
+
+
+class rt_field[T, O](constructed_field):
+    """
+    rt_fields (runtime_fields) are the last fields excecuted before the
+    __preinit__ and __postinit__ functions are called.
+    It gives the function passed to it access to the node instance.
+    This is useful to do construction that depends on parameters passed by __init__.
+    """
+
     def __init__(self, fget: Callable[[T], O]) -> None:
         super().__init__()
         self.func = fget
         self.lookup: dict[T, O] = {}
 
-    def _construct(self, obj: T):
+    def __construct__(self, obj: T):
         constructed = self.func(obj)
         # TODO find a better way for this
         # in python 3.13 name support
@@ -145,6 +169,14 @@ def __init__(self, node: "Node", other: "Node", *args: object) -> None:
 class NodeNoParent(NodeException): ...
 
 
+class InitVar(dataclass_InitVar):
+    """
+    This is a type-marker which instructs the Node constructor to ignore the field.
+
+    Inspired by dataclasses.InitVar, which it inherits from.
+    """
+
+
 # -----------------------------------------------------------------------------
 
 
@@ -167,7 +199,7 @@ def __hash__(self) -> int:
         # TODO proper hash
         return hash(id(self))
 
-    def add[T: Node](
+    def add[T: Node | GraphInterface](
         self,
         obj: T,
         name: str | None = None,
@@ -176,9 +208,10 @@ def add[T: Node](
         assert obj is not None
 
         if container is None:
-            container = self.runtime_anon
             if name:
                 container = self.runtime
+            else:
+                container = self.runtime_anon
 
         try:
             container_name = find(vars(self).items(), lambda x: x[1] is container)[0]
@@ -197,7 +230,11 @@ def add[T: Node](
             container.append(obj)
             name = f"{container_name}[{len(container) - 1}]"
 
-        self._handle_add_node(name, obj)
+        if isinstance(obj, GraphInterface):
+            self._handle_add_gif(name, obj)
+        else:
+            self._handle_add_node(name, obj)
+
         return obj
 
     def add_to_container[T: Node](
@@ -230,8 +267,64 @@ def all_anno(cls):
 
         LL_Types = (Node, GraphInterface)
 
-        annos = all_anno(cls)
-        vars_ = all_vars(cls)
+        vars_ = {
+            name: obj
+            for name, obj in all_vars(cls).items()
+            # private fields are always ignored
+            if not name.startswith("_")
+            # only consider fab_fields
+            and isinstance(obj, fab_field)
+        }
+        annos = {
+            name: obj
+            for name, obj in all_anno(cls).items()
+            # private fields are always ignored
+            if not name.startswith("_")
+            # explicitly ignore InitVars
+            and not isinstance(obj, InitVar)
+            # variables take precedence over annos
+            and name not in vars_
+        }
+
+        # ensure no field annotations are a property
+        # If properties are constructed as instance fields, their
+        # getters and setters aren't called when assigning to them.
+        #
+        # This means we won't actually construct the underlying graph properly.
+        # It's pretty insidious because it's completely non-obvious that we're
+        # missing these graph connections.
+        # TODO: make this an exception group instead
+        for name, obj in annos.items():
+            if (origin := get_origin(obj)) is not None:
+                # you can't truly subclass properties because they're a descriptor
+                # type, so instead we check if the origin is a property via our fields
+                if issubclass(origin, constructed_field):
+                    raise FieldError(
+                        f"{name} is a property, which cannot be created from a field "
+                        "annotation. Please instantiate the field directly."
+                    )
+
+        # FIXME: something's fucked up in the new version of this,
+        # but I can't for the life of me figure out what
+        clsfields_unf_new = dict(chain(annos.items(), vars_.items()))
+        clsfields_unf_old = {
+            name: obj
+            for name, obj in chain(
+                (
+                    (name, obj)
+                    for name, obj in all_anno(cls).items()
+                    if not isinstance(obj, InitVar)
+                ),
+                (
+                    (name, f)
+                    for name, f in all_vars(cls).items()
+                    if isinstance(f, fab_field)
+                ),
+            )
+            if not name.startswith("_")
+        }
+        assert clsfields_unf_old == clsfields_unf_new
+        clsfields_unf = clsfields_unf_old
 
         def is_node_field(obj):
             def is_genalias_node(obj):
@@ -260,20 +353,11 @@ def is_genalias_node(obj):
             if get_origin(obj):
                 return is_genalias_node(obj)
 
-            if isinstance(obj, rt_field):
+            if isinstance(obj, constructed_field):
                 return True
 
             return False
 
-        clsfields_unf = {
-            name: obj
-            for name, obj in chain(
-                [(name, f) for name, f in annos.items()],
-                [(name, f) for name, f in vars_.items() if isinstance(f, fab_field)],
-            )
-            if not name.startswith("_")
-        }
-
         nonfabfields, fabfields = partition(
             lambda x: is_node_field(x[1]), clsfields_unf.items()
         )
@@ -305,7 +389,9 @@ def handle_add(name, obj):
             elif isinstance(obj, Node):
                 self._handle_add_node(name, obj)
             else:
-                assert False
+                raise TypeError(
+                    f"Cannot handle adding field {name=} of type {type(obj)}"
+                )
 
         def append(name, inst):
             if isinstance(inst, LL_Types):
@@ -321,21 +407,15 @@ def append(name, inst):
             return inst
 
         def _setup_field(name, obj):
-            def setup_gen_alias(name, obj):
-                origin = get_origin(obj)
-                assert origin
+            if isinstance(obj, str):
+                raise NotImplementedError()
+
+            if (origin := get_origin(obj)) is not None:
                 if isinstance(origin, type):
                     setattr(self, name, append(name, origin()))
                     return
                 raise NotImplementedError(origin)
 
-            if isinstance(obj, str):
-                raise NotImplementedError()
-
-            if get_origin(obj):
-                setup_gen_alias(name, obj)
-                return
-
             if isinstance(obj, _d_field):
                 setattr(self, name, append(name, obj.default_factory()))
                 return
@@ -344,8 +424,9 @@ def setup_gen_alias(name, obj):
                 setattr(self, name, append(name, obj()))
                 return
 
-            if isinstance(obj, rt_field):
-                append(name, obj._construct(self))
+            if isinstance(obj, constructed_field):
+                if (constructed := obj.__construct__(self)) is not None:
+                    append(name, constructed)
                 return
 
             raise NotImplementedError()
@@ -354,13 +435,19 @@ def setup_field(name, obj):
             try:
                 _setup_field(name, obj)
             except Exception as e:
+                # this is a bit of a hack to provide complete context to debuggers
+                # for underlying field construction errors
+                if debugging():
+                    raise
                 raise FieldConstructionError(
                     self,
                     name,
                     f'An exception occurred while constructing field "{name}"',
                 ) from e
 
-        nonrt, rt = partition(lambda x: isinstance(x[1], rt_field), clsfields.items())
+        nonrt, rt = partition(
+            lambda x: isinstance(x[1], constructed_field), clsfields.items()
+        )
         for name, obj in nonrt:
             setup_field(name, obj)