From 58571a09491e99422370685b35d9af5674fe7464 Mon Sep 17 00:00:00 2001 From: Licini Date: Wed, 2 Apr 2025 09:38:37 +0200 Subject: [PATCH 01/19] add scene object factory --- src/compas/scene/__init__.py | 2 ++ src/compas/scene/scene.py | 6 ++++- src/compas/scene/sceneobject.py | 47 +++++++++++++++++++++++++++++---- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/compas/scene/__init__.py b/src/compas/scene/__init__.py index 8aaa2c03581d..0c5285c8f7de 100644 --- a/src/compas/scene/__init__.py +++ b/src/compas/scene/__init__.py @@ -10,6 +10,7 @@ from .exceptions import SceneObjectNotRegisteredError from .sceneobject import SceneObject +from .sceneobject import SceneObjectFactory from .meshobject import MeshObject from .graphobject import GraphObject from .geometryobject import GeometryObject @@ -43,6 +44,7 @@ def register_scene_objects_base(): __all__ = [ "SceneObjectNotRegisteredError", "SceneObject", + "SceneObjectFactory", "MeshObject", "GraphObject", "GeometryObject", diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 39f56499a6b8..632c360afa94 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -10,6 +10,7 @@ from .context import detect_current_context from .group import Group from .sceneobject import SceneObject +from .sceneobject import SceneObjectFactory class Scene(Tree): @@ -117,7 +118,10 @@ def add(self, item, parent=None, **kwargs): if kwargs["context"] != self.context: raise Exception("Object context should be the same as scene context: {} != {}".format(kwargs["context"], self.context)) del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error - sceneobject = SceneObject(item=item, context=self.context, **kwargs) # type: ignore + + # Use the factory to create the scene object + sceneobject = SceneObjectFactory.create(item=item, context=self.context, **kwargs) + super(Scene, self).add(sceneobject, parent=parent) return sceneobject diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index dbfcd6064636..dca5b9e8de61 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -22,6 +22,45 @@ from .descriptors.protocol import DescriptorProtocol +class SceneObjectFactory: + """Factory class for creating appropriate SceneObject instances based on input item type. + + This factory encapsulates the logic for selecting the right SceneObject subclass + for a given data item, making the creation process more explicit and easier to understand. + """ + + @staticmethod + def create(item=None, **kwargs): + """Create appropriate SceneObject instance based on item type. + + Parameters + ---------- + item : :class:`compas.data.Data` + The data item to create a scene object for. + **kwargs : dict + Additional keyword arguments to pass to the SceneObject constructor. + + Returns + ------- + :class:`compas.scene.SceneObject` + A SceneObject instance of the appropriate subclass for the given item. + + Raises + ------ + ValueError + If item is None. + SceneObjectNotRegisteredError + If no scene object is registered for the item type in the current context. + """ + if item is None: + raise ValueError("Cannot create a scene object for None. Please ensure you pass an instance of a supported class.") + + sceneobject_cls = get_sceneobject_cls(item, **kwargs) + + # Create and return an instance of the appropriate scene object class + return sceneobject_cls(item=item, **kwargs) + + class SceneObject(TreeNode): """Base class for all scene objects. @@ -84,10 +123,6 @@ class SceneObject(TreeNode): color = ColorAttribute() - def __new__(cls, item=None, **kwargs): - sceneobject_cls = get_sceneobject_cls(item, **kwargs) - return super(SceneObject, cls).__new__(sceneobject_cls) - def __init__( self, item=None, # type: compas.data.Data | None @@ -244,7 +279,9 @@ def add(self, item, **kwargs): if kwargs["context"] != self.context: raise Exception("Child context should be the same as parent context: {} != {}".format(kwargs["context"], self.context)) del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error - sceneobject = SceneObject(item=item, context=self.context, **kwargs) # type: ignore + + # Use the factory to create the scene object + sceneobject = SceneObjectFactory.create(item=item, context=self.context, **kwargs) super(SceneObject, self).add(sceneobject) return sceneobject From eff2b2e9ed46b7dde01dc605376e6f07bab0057f Mon Sep 17 00:00:00 2001 From: Licini Date: Wed, 2 Apr 2025 10:57:58 +0200 Subject: [PATCH 02/19] add node_repr option to tree --- src/compas/datastructures/tree/tree.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/compas/datastructures/tree/tree.py b/src/compas/datastructures/tree/tree.py index 458dfda9532e..0abf3e48f7a9 100644 --- a/src/compas/datastructures/tree/tree.py +++ b/src/compas/datastructures/tree/tree.py @@ -412,7 +412,7 @@ def get_node_by_name(self, name): """ for node in self.nodes: - if node.name == name: + if str(node.name) == str(name): return node def get_nodes_by_name(self, name): @@ -436,7 +436,7 @@ def get_nodes_by_name(self, name): nodes.append(node) return nodes - def get_hierarchy_string(self, max_depth=None): + def get_hierarchy_string(self, max_depth=None, node_repr=None): """ Return string representation for the spatial hierarchy of the tree. @@ -445,6 +445,10 @@ def get_hierarchy_string(self, max_depth=None): max_depth : int, optional The maximum depth of the hierarchy to print. Default is ``None``, in which case the entire hierarchy is printed. + node_repr : callable, optional + A callable to represent the node string. + Default is ``None``, in which case the node.__repr__() is used. + Returns ------- @@ -455,17 +459,22 @@ def get_hierarchy_string(self, max_depth=None): hierarchy = [] - def traverse(node, hierarchy, prefix="", last=True, depth=0): + def traverse(node, hierarchy, prefix="", last=True, depth=0, node_repr=None): if max_depth is not None and depth > max_depth: return + if node_repr is None: + node_string = node.__repr__() + else: + node_string = node_repr(node) + connector = "└── " if last else "├── " - hierarchy.append("{}{}{}".format(prefix, connector, node)) + hierarchy.append("{}{}{}".format(prefix, connector, node_string)) prefix += " " if last else "│ " for i, child in enumerate(node.children): - traverse(child, hierarchy, prefix, i == len(node.children) - 1, depth + 1) + traverse(child, hierarchy, prefix, i == len(node.children) - 1, depth + 1, node_repr) - traverse(self.root, hierarchy) + traverse(self.root, hierarchy, node_repr=node_repr) return "\n".join(hierarchy) From 3d5e493955c91f2af97be9af75b181cdf325bd3e Mon Sep 17 00:00:00 2001 From: Licini Date: Wed, 2 Apr 2025 11:06:26 +0200 Subject: [PATCH 03/19] use "data store" for Scene --- src/compas/scene/scene.py | 80 ++++++++++++------------- src/compas/scene/sceneobject.py | 101 +++++++++----------------------- 2 files changed, 66 insertions(+), 115 deletions(-) diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 632c360afa94..e17cf45f6cf7 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -1,6 +1,7 @@ import compas.data # noqa: F401 import compas.datastructures # noqa: F401 import compas.geometry # noqa: F401 +from compas.datastructures import Datastructure from compas.datastructures import Tree from compas.datastructures import TreeNode @@ -13,7 +14,7 @@ from .sceneobject import SceneObjectFactory -class Scene(Tree): +class Scene(Datastructure): """A scene is a container for hierarchical scene objects which are to be visualised in a given context. Parameters @@ -44,49 +45,42 @@ class Scene(Tree): @property def __data__(self): # type: () -> dict - items = {str(object.item.guid): object.item for object in self.objects if object.item is not None} return { "name": self.name, - "root": self.root.__data__, # type: ignore - "items": list(items.values()), + "items": list(self.items.values()), + "objects": list(self.objects.values()), + "tree": self.tree, } - @classmethod - def __from_data__(cls, data): - # type: (dict) -> Scene - scene = cls(data["name"]) - items = {str(item.guid): item for item in data["items"]} - - def add(node, parent, items): - for child_node in node.get("children", []): - settings = child_node["settings"] - if "item" in child_node: - guid = child_node["item"] - sceneobject = parent.add(items[guid], **settings) - else: - sceneobject = parent.add(Group(**settings)) - add(child_node, sceneobject, items) - - add(data["root"], scene, items) - - return scene - def __init__(self, name="Scene", context=None): # type: (str, str | None) -> None super(Scene, self).__init__(name=name) - super(Scene, self).add(TreeNode(name="ROOT")) + + # TODO: deal with context self.context = context or detect_current_context() - @property - def objects(self): - # type: () -> list[SceneObject] - return [node for node in self.nodes if not node.is_root] # type: ignore + self.tree = Tree() + self.tree.add(TreeNode(name="ROOT")) + self.items = {} + self.objects = {} + + def __repr__(self): + # type: () -> str + + def node_repr(node): + if node.name == "ROOT": + return self.name + + sceneobject = self.objects[node.name] + return str(sceneobject) + + return self.tree.get_hierarchy_string(node_repr=node_repr) @property def context_objects(self): # type: () -> list guids = [] - for obj in self.objects: + for obj in self.objects.values(): guids += obj.guids return guids @@ -109,20 +103,22 @@ def add(self, item, parent=None, **kwargs): The scene object associated with the item. """ - parent = parent or self.root + # Create a corresponding new scene object + sceneobject = SceneObjectFactory.create(item=item, context=self.context, scene=self, **kwargs) - if isinstance(item, SceneObject): - sceneobject = item + # Add the scene object and item to the data store + self.objects[str(sceneobject.guid)] = sceneobject + self.items[str(item.guid)] = item + + # Add the scene object to the hierarchical tree + if parent is None: + parent_node = self.tree.root else: - if "context" in kwargs: - if kwargs["context"] != self.context: - raise Exception("Object context should be the same as scene context: {} != {}".format(kwargs["context"], self.context)) - del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error - - # Use the factory to create the scene object - sceneobject = SceneObjectFactory.create(item=item, context=self.context, **kwargs) - - super(Scene, self).add(sceneobject, parent=parent) + print(parent.guid) + parent_node = self.tree.get_node_by_name(parent.guid) + + self.tree.add(TreeNode(name=str(sceneobject.guid)), parent=parent_node) + return sceneobject def clear_context(self, guids=None): diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index dca5b9e8de61..064d7e2ba49a 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -24,27 +24,27 @@ class SceneObjectFactory: """Factory class for creating appropriate SceneObject instances based on input item type. - + This factory encapsulates the logic for selecting the right SceneObject subclass for a given data item, making the creation process more explicit and easier to understand. """ - + @staticmethod - def create(item=None, **kwargs): + def create(item=None, scene=None, **kwargs): """Create appropriate SceneObject instance based on item type. - + Parameters ---------- item : :class:`compas.data.Data` The data item to create a scene object for. **kwargs : dict Additional keyword arguments to pass to the SceneObject constructor. - + Returns ------- :class:`compas.scene.SceneObject` A SceneObject instance of the appropriate subclass for the given item. - + Raises ------ ValueError @@ -55,13 +55,16 @@ def create(item=None, **kwargs): if item is None: raise ValueError("Cannot create a scene object for None. Please ensure you pass an instance of a supported class.") + if scene is None: + raise ValueError("Cannot create a scene object without a scene.") + sceneobject_cls = get_sceneobject_cls(item, **kwargs) - + # Create and return an instance of the appropriate scene object class - return sceneobject_cls(item=item, **kwargs) + return sceneobject_cls(item=item, scene=scene, **kwargs) -class SceneObject(TreeNode): +class SceneObject(Data): """Base class for all scene objects. Parameters @@ -130,9 +133,9 @@ def __init__( color=None, # type: compas.colors.Color | None opacity=1.0, # type: float show=True, # type: bool - frame=None, # type: compas.geometry.Frame | None transformation=None, # type: compas.geometry.Transformation | None context=None, # type: str | None + scene=None, # type: compas.scene.Scene | None **kwargs # type: dict ): # fmt: skip # type: (...) -> None @@ -145,7 +148,8 @@ def __init__( # because it has no access to the tree and/or the scene before it is added # which means that adding child objects will be added in context "None" self.context = context - self._item = item + self._scene = scene + self._item = item.guid self._guids = [] self._node = None self._transformation = transformation @@ -158,16 +162,14 @@ def __init__( def __data__(self): # type: () -> dict return { - "item": str(self.item.guid), - "settings": self.settings, - "children": [child.__data__ for child in self.children], + "item": str(self._item), + "name": self.name, + "color": self.color, + "opacity": self.opacity, + "show": self.show, + "transformation": self.transformation, } - @classmethod - def __from_data__(cls, data): - # type: (dict) -> None - raise TypeError("Serialisation outside Scene not allowed.") - def __repr__(self): # type: () -> str return "<{}: {}>".format(self.__class__.__name__, self.name) @@ -175,12 +177,17 @@ def __repr__(self): @property def scene(self): # type: () -> compas.scene.Scene | None - return self.tree + return self._scene @property def item(self): # type: () -> compas.data.Data - return self._item + return self.scene.items[self._item] + + @property + def node(self): + # type: () -> compas.datastructures.TreeNode + return self.scene.tree.get_object_node(self.guid) @property def guids(self): @@ -234,58 +241,6 @@ def contrastcolor(self, color): # type: (compas.colors.Color) -> None self._contrastcolor = Color.coerce(color) - @property - def settings(self): - # type: () -> dict - settings = { - "name": self.name, - "color": self.color, - "opacity": self.opacity, - "show": self.show, - } - - if self.frame: - settings["frame"] = self.frame - if self.transformation: - settings["transformation"] = self.transformation - - return settings - - def add(self, item, **kwargs): - # type: (compas.data.Data, dict) -> SceneObject - """Add a child item to the scene object. - - Parameters - ---------- - item : :class:`compas.data.Data` - The item to add. - **kwargs : dict - Additional keyword arguments to create the scene object for the item. - - Returns - ------- - :class:`compas.scene.SceneObject` - The scene object associated with the added item. - - Raises - ------ - ValueError - If the scene object does not have an associated scene node. - """ - if isinstance(item, SceneObject): - sceneobject = item - else: - if "context" in kwargs: - if kwargs["context"] != self.context: - raise Exception("Child context should be the same as parent context: {} != {}".format(kwargs["context"], self.context)) - del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error - - # Use the factory to create the scene object - sceneobject = SceneObjectFactory.create(item=item, context=self.context, **kwargs) - - super(SceneObject, self).add(sceneobject) - return sceneobject - def draw(self): """The main drawing method.""" raise NotImplementedError From 10e42256da232f27b0e3be848b0a76292a70e329 Mon Sep 17 00:00:00 2001 From: Licini Date: Wed, 14 May 2025 13:02:53 +0200 Subject: [PATCH 04/19] finish the work, update tests --- src/compas/scene/group.py | 13 ----- src/compas/scene/scene.py | 68 ++++++++++++++++---------- src/compas/scene/sceneobject.py | 82 ++++++++++++++++++++++++++++---- tests/compas/scene/test_scene.py | 13 ++--- 4 files changed, 125 insertions(+), 51 deletions(-) diff --git a/src/compas/scene/group.py b/src/compas/scene/group.py index af10b42d514e..7c02ccb9955a 100644 --- a/src/compas/scene/group.py +++ b/src/compas/scene/group.py @@ -27,16 +27,3 @@ class Group(SceneObject): └── """ - - def __new__(cls, *args, **kwargs): - # overwriting __new__ to revert to the default behavior of normal object, So an instance can be created directly without providing a registered item. - return object.__new__(cls) - - @property - def __data__(self): - # type: () -> dict - data = { - "settings": self.settings, - "children": [child.__data__ for child in self.children], - } - return data diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index e17cf45f6cf7..30f166e4180e 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -9,7 +9,6 @@ from .context import before_draw from .context import clear from .context import detect_current_context -from .group import Group from .sceneobject import SceneObject from .sceneobject import SceneObjectFactory @@ -47,32 +46,33 @@ def __data__(self): # type: () -> dict return { "name": self.name, - "items": list(self.items.values()), - "objects": list(self.objects.values()), + "attributes": self.attributes, + "datastore": self.datastore, + "objects": self.objects, "tree": self.tree, } - def __init__(self, name="Scene", context=None): - # type: (str, str | None) -> None - super(Scene, self).__init__(name=name) + def __init__(self, context=None, datastore=None, objects=None, tree=None, **kwargs): + # type: (str | None, dict | None, dict | None, Tree | None, **kwargs) -> None + super(Scene, self).__init__(**kwargs) - # TODO: deal with context self.context = context or detect_current_context() - - self.tree = Tree() - self.tree.add(TreeNode(name="ROOT")) - self.items = {} - self.objects = {} + self.datastore = datastore or {} + self.objects = objects or {} + self.tree = tree or Tree() + if self.tree.root is None: + self.tree.add(TreeNode(name=self.name)) def __repr__(self): # type: () -> str def node_repr(node): - if node.name == "ROOT": - return self.name - - sceneobject = self.objects[node.name] - return str(sceneobject) + # type: (TreeNode) -> str + if node.is_root: + return node.name + else: + sceneobject = self.objects[node.name] + return str(sceneobject) return self.tree.get_hierarchy_string(node_repr=node_repr) @@ -108,19 +108,39 @@ def add(self, item, parent=None, **kwargs): # Add the scene object and item to the data store self.objects[str(sceneobject.guid)] = sceneobject - self.items[str(item.guid)] = item + self.datastore[str(item.guid)] = item # Add the scene object to the hierarchical tree if parent is None: parent_node = self.tree.root else: - print(parent.guid) + if not isinstance(parent, SceneObject): + raise ValueError("Parent is not a SceneObject.", parent) parent_node = self.tree.get_node_by_name(parent.guid) + if parent_node is None: + raise ValueError("Parent is not part of the scene.", parent) self.tree.add(TreeNode(name=str(sceneobject.guid)), parent=parent_node) return sceneobject + def remove(self, sceneobject): + """Remove a scene object along with all its descendants from the scene. + + Parameters + ---------- + sceneobject : :class:`compas.scene.SceneObject` + The scene object to remove. + """ + # type: (SceneObject) -> None + guid = str(sceneobject.guid) + self.objects.pop(guid, None) + node = self.tree.get_node_by_name(guid) + if node: + for descendant in node.descendants: + self.objects.pop(descendant.name, None) + self.tree.remove(node) + def clear_context(self, guids=None): # type: (list | None) -> None """Clear the visualisation context. @@ -173,7 +193,7 @@ def clear(self, clear_scene=True, clear_context=True): """ guids = [] - for sceneobject in self.objects: + for sceneobject in list(self.objects.values()): guids += sceneobject.guids sceneobject._guids = None @@ -233,7 +253,7 @@ def find_by_name(self, name): return self.get_node_by_name(name=name) def find_by_itemtype(self, itemtype): - # type: (...) -> SceneObject | None + # type: (type) -> SceneObject | None """Find the first scene object with a data item of the given type. Parameters @@ -246,12 +266,12 @@ def find_by_itemtype(self, itemtype): :class:`SceneObject` or None """ - for obj in self.objects: + for obj in self.objects.values(): if isinstance(obj.item, itemtype): return obj def find_all_by_itemtype(self, itemtype): - # type: (...) -> list[SceneObject] + # type: (type) -> list[SceneObject] """Find all scene objects with a data item of the given type. Parameters @@ -265,7 +285,7 @@ def find_all_by_itemtype(self, itemtype): """ sceneobjects = [] - for obj in self.objects: + for obj in self.objects.values(): if isinstance(obj.item, itemtype): sceneobjects.append(obj) return sceneobjects diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index cae19d4d8f0c..43052e34ca87 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -12,7 +12,6 @@ import compas.scene # noqa: F401 from compas.colors import Color from compas.data import Data -from compas.datastructures import TreeNode from compas.geometry import Frame from compas.geometry import Transformation @@ -55,8 +54,9 @@ def create(item=None, scene=None, **kwargs): if item is None: raise ValueError("Cannot create a scene object for None. Please ensure you pass an instance of a supported class.") - if scene is None: - raise ValueError("Cannot create a scene object without a scene.") + if isinstance(item, SceneObject): + item._scene = scene + return item sceneobject_cls = get_sceneobject_cls(item, **kwargs) @@ -139,17 +139,24 @@ def __init__( **kwargs # type: dict ): # fmt: skip # type: (...) -> None - if item and not isinstance(item, Data): - raise ValueError("The item assigned to this scene object should be a data object: {}".format(type(item))) name = name or getattr(item, "name", None) super(SceneObject, self).__init__(name=name, **kwargs) # the scene object needs to store the context # because it has no access to the tree and/or the scene before it is added # which means that adding child objects will be added in context "None" + + if isinstance(item, Data): + self._item = str(item.guid) + elif isinstance(item, str): + self._item = item + elif item is None: + self._item = None + else: + raise ValueError("The item assigned to this scene object should be a data object or a str guid: {}".format(item)) + self.context = context self._scene = scene - self._item = item.guid self._guids = [] self._node = None self._transformation = transformation @@ -162,7 +169,7 @@ def __init__( def __data__(self): # type: () -> dict return { - "item": str(self._item), + "item": self._item, "name": self.name, "color": self.color, "opacity": self.opacity, @@ -187,7 +194,44 @@ def item(self): @property def node(self): # type: () -> compas.datastructures.TreeNode - return self.scene.tree.get_object_node(self.guid) + return self.scene.tree.get_node_by_name(self.guid) + + @property + def is_root(self): + # type: () -> bool + return self.node.is_root + + @property + def is_leaf(self): + # type: () -> bool + return self.node.is_leaf + + @property + def is_branch(self): + # type: () -> bool + return self.node.is_branch + + @property + def parentnode(self): + # type: () -> compas.datastructures.Node | None + return self.node.parent + + @property + def childnodes(self): + # type: () -> list[compas.datastructures.Node] + return self.node.children + + @property + def parent(self): + # type: () -> compas.scene.SceneObject | None + if self.parentnode and not self.parentnode.is_root: + return self.scene.objects[self.parentnode.name] + return None + + @property + def children(self): + # type: () -> list[compas.scene.SceneObject] + return [self.scene.objects[child.name] for child in self.childnodes] @property def guids(self): @@ -236,6 +280,28 @@ def contrastcolor(self): self._contrastcolor = self.color.lightened(50) return self._contrastcolor + def add(self, item, **kwargs): + """Add a scene object to the scene. + + Parameters + ---------- + item : :class:`compas.data.Data` + The item to add to the scene. + **kwargs : dict + Additional keyword arguments to pass to the SceneObject constructor. + + Returns + ------- + :class:`compas.scene.SceneObject` + The added scene object. + """ + # type: (compas.data.Data, dict) -> compas.scene.SceneObject + return self.scene.add(item, parent=self, **kwargs) + + def remove(self): + """Remove this scene object along with all its descendants from the scene.""" + self.scene.remove(self) + @contrastcolor.setter def contrastcolor(self, color): # type: (compas.colors.Color) -> None diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index ce7f42a7ee90..31106876ca37 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -6,6 +6,7 @@ from compas.scene import register from compas.scene import Scene from compas.scene import SceneObject + from compas.scene import SceneObjectFactory from compas.scene import SceneObjectNotRegisteredError from compas.data import Data from compas.geometry import Box @@ -46,29 +47,29 @@ def test_get_sceneobject_cls_with_orderly_registration(): register(FakeItem, FakeSceneObject, context="fake") register(FakeSubItem, FakeSubSceneObject, context="fake") item = FakeItem() - sceneobject = SceneObject(item, context="fake") + sceneobject = SceneObjectFactory.create(item, context="fake") assert isinstance(sceneobject, FakeSceneObject) item = FakeSubItem() - sceneobject = SceneObject(item, context="fake") + sceneobject = SceneObjectFactory.create(item, context="fake") assert isinstance(sceneobject, FakeSubSceneObject) def test_get_sceneobject_cls_with_out_of_order_registration(): register(FakeSubItem, FakeSubSceneObject, context="fake") register(FakeItem, FakeSceneObject, context="fake") item = FakeItem() - sceneobject = SceneObject(item, context="fake") + sceneobject = SceneObjectFactory.create(item, context="fake") assert isinstance(sceneobject, FakeSceneObject) item = FakeSubItem() - sceneobject = SceneObject(item, context="fake") + sceneobject = SceneObjectFactory.create(item, context="fake") assert isinstance(sceneobject, FakeSubSceneObject) def test_sceneobject_auto_context_discovery(mocker): register_fake_context() item = FakeItem() - sceneobject = SceneObject(item) + sceneobject = SceneObjectFactory.create(item) assert isinstance(sceneobject, FakeSceneObject) @@ -78,7 +79,7 @@ def test_sceneobject_auto_context_discovery_no_context(mocker): with pytest.raises(SceneObjectNotRegisteredError): item = FakeSubItem() - _ = SceneObject(item) + _ = SceneObjectFactory.create(item) def test_sceneobject_transform(): scene = Scene() From e8b5fabf1f5d7ba81267499d9d468510b4fc43db Mon Sep 17 00:00:00 2001 From: Licini Date: Wed, 14 May 2025 13:09:14 +0200 Subject: [PATCH 05/19] put back setter --- src/compas/scene/sceneobject.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 43052e34ca87..6cefe34bc94d 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -280,6 +280,11 @@ def contrastcolor(self): self._contrastcolor = self.color.lightened(50) return self._contrastcolor + @contrastcolor.setter + def contrastcolor(self, color): + # type: (compas.colors.Color) -> None + self._contrastcolor = Color.coerce(color) + def add(self, item, **kwargs): """Add a scene object to the scene. From 82fa199480eec3b6b3e466fa5fcb7ec63a5a66b5 Mon Sep 17 00:00:00 2001 From: Licini Date: Wed, 14 May 2025 13:26:45 +0200 Subject: [PATCH 06/19] objectstore --- src/compas/scene/scene.py | 32 +++++++++++++++++++++----------- src/compas/scene/sceneobject.py | 4 ++-- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 30f166e4180e..592456217195 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -48,17 +48,17 @@ def __data__(self): "name": self.name, "attributes": self.attributes, "datastore": self.datastore, - "objects": self.objects, + "objectstore": self.objectstore, "tree": self.tree, } - def __init__(self, context=None, datastore=None, objects=None, tree=None, **kwargs): + def __init__(self, context=None, datastore=None, objectstore=None, tree=None, **kwargs): # type: (str | None, dict | None, dict | None, Tree | None, **kwargs) -> None super(Scene, self).__init__(**kwargs) self.context = context or detect_current_context() self.datastore = datastore or {} - self.objects = objects or {} + self.objectstore = objectstore or {} self.tree = tree or Tree() if self.tree.root is None: self.tree.add(TreeNode(name=self.name)) @@ -71,16 +71,26 @@ def node_repr(node): if node.is_root: return node.name else: - sceneobject = self.objects[node.name] + sceneobject = self.objectstore[node.name] return str(sceneobject) return self.tree.get_hierarchy_string(node_repr=node_repr) + @property + def items(self): + # type: () -> list + return list(self.datastore.values()) + + @property + def objects(self): + # type: () -> list + return list(self.objectstore.values()) + @property def context_objects(self): # type: () -> list guids = [] - for obj in self.objects.values(): + for obj in self.objects: guids += obj.guids return guids @@ -107,7 +117,7 @@ def add(self, item, parent=None, **kwargs): sceneobject = SceneObjectFactory.create(item=item, context=self.context, scene=self, **kwargs) # Add the scene object and item to the data store - self.objects[str(sceneobject.guid)] = sceneobject + self.objectstore[str(sceneobject.guid)] = sceneobject self.datastore[str(item.guid)] = item # Add the scene object to the hierarchical tree @@ -134,11 +144,11 @@ def remove(self, sceneobject): """ # type: (SceneObject) -> None guid = str(sceneobject.guid) - self.objects.pop(guid, None) + self.objectstore.pop(guid, None) node = self.tree.get_node_by_name(guid) if node: for descendant in node.descendants: - self.objects.pop(descendant.name, None) + self.objectstore.pop(descendant.name, None) self.tree.remove(node) def clear_context(self, guids=None): @@ -193,7 +203,7 @@ def clear(self, clear_scene=True, clear_context=True): """ guids = [] - for sceneobject in list(self.objects.values()): + for sceneobject in self.objects: guids += sceneobject.guids sceneobject._guids = None @@ -266,7 +276,7 @@ def find_by_itemtype(self, itemtype): :class:`SceneObject` or None """ - for obj in self.objects.values(): + for obj in self.objects: if isinstance(obj.item, itemtype): return obj @@ -285,7 +295,7 @@ def find_all_by_itemtype(self, itemtype): """ sceneobjects = [] - for obj in self.objects.values(): + for obj in self.objects: if isinstance(obj.item, itemtype): sceneobjects.append(obj) return sceneobjects diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 6cefe34bc94d..2bfb44484966 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -225,13 +225,13 @@ def childnodes(self): def parent(self): # type: () -> compas.scene.SceneObject | None if self.parentnode and not self.parentnode.is_root: - return self.scene.objects[self.parentnode.name] + return self.scene.objectstore[self.parentnode.name] return None @property def children(self): # type: () -> list[compas.scene.SceneObject] - return [self.scene.objects[child.name] for child in self.childnodes] + return [self.scene.objectstore[child.name] for child in self.childnodes] @property def guids(self): From b730a4d8b720d0edb771948eb3b624d6d2bbcc73 Mon Sep 17 00:00:00 2001 From: Licini Date: Wed, 14 May 2025 13:42:16 +0200 Subject: [PATCH 07/19] fixes --- src/compas/scene/scene.py | 11 ++++++++--- src/compas/scene/sceneobject.py | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 592456217195..395922678568 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -78,17 +78,17 @@ def node_repr(node): @property def items(self): - # type: () -> list + # type: () -> list[compas.data.Data] return list(self.datastore.values()) @property def objects(self): - # type: () -> list + # type: () -> list[SceneObject] return list(self.objectstore.values()) @property def context_objects(self): - # type: () -> list + # type: () -> list[SceneObject] guids = [] for obj in self.objects: guids += obj.guids @@ -113,6 +113,11 @@ def add(self, item, parent=None, **kwargs): The scene object associated with the item. """ + if "context" in kwargs: + if kwargs["context"] != self.context: + raise Exception("Object context should be the same as scene context: {} != {}".format(kwargs["context"], self.context)) + del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error + # Create a corresponding new scene object sceneobject = SceneObjectFactory.create(item=item, context=self.context, scene=self, **kwargs) diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 2bfb44484966..42e909f5b872 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -286,12 +286,12 @@ def contrastcolor(self, color): self._contrastcolor = Color.coerce(color) def add(self, item, **kwargs): - """Add a scene object to the scene. + """Add a child item to the scene object. Parameters ---------- item : :class:`compas.data.Data` - The item to add to the scene. + The item to add. **kwargs : dict Additional keyword arguments to pass to the SceneObject constructor. From 0c866d12dbfd69be2b99776518108da6d2f1d8ef Mon Sep 17 00:00:00 2001 From: Licini Date: Thu, 15 May 2025 10:21:11 +0200 Subject: [PATCH 08/19] clean up the function --- src/compas/scene/context.py | 59 +++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/src/compas/scene/context.py b/src/compas/scene/context.py index d083023fa0fd..f814e12af4a2 100644 --- a/src/compas/scene/context.py +++ b/src/compas/scene/context.py @@ -119,36 +119,57 @@ def detect_current_context(): return None -def _get_sceneobject_cls(data, **kwargs): - # in any case user gets to override the choice - context_name = kwargs.get("context") or detect_current_context() +def get_sceneobject_cls(item, context=None, **kwargs): + """Get the scene object class for a given item in the current context. If no context is provided, the current context is detected. + If the exact item type is not registered, a closest match in its inheritance hierarchy is used. - dtype = type(data) + Parameters + ---------- + item : :class:`~compas.data.Data` + The item to get the scene object class for. + context : Literal['Viewer', 'Rhino', 'Grasshopper', 'Blender'], optional + The visualization context in which the pair should be registered. + **kwargs : dict + Additional keyword arguments. + + Raises + ------ + ValueError + If the item is None. + SceneObjectNotRegisteredError + If no scene object is registered for the item type in the current context. + + Returns + ------- + :class:`~compas.scene.SceneObject` + The scene object class for the given item. + + """ + + if item is None: + raise ValueError("Cannot create a scene object for None. Please ensure you pass a instance of a supported class.") + + if not ITEM_SCENEOBJECT: + register_scene_objects() + + if context is None: + context = detect_current_context() + + itemtype = type(item) cls = None if "sceneobject_type" in kwargs: cls = kwargs["sceneobject_type"] else: - context = ITEM_SCENEOBJECT[context_name] + context = ITEM_SCENEOBJECT[context] - for type_ in inspect.getmro(dtype): - cls = context.get(type_, None) + for inheritancetype in inspect.getmro(itemtype): + cls = context.get(inheritancetype, None) if cls is not None: break if cls is None: - raise SceneObjectNotRegisteredError("No scene object is registered for this data type: {} in this context: {}".format(dtype, context_name)) - - return cls - - -def get_sceneobject_cls(item, **kwargs): - if not ITEM_SCENEOBJECT: - register_scene_objects() - - if item is None: - raise ValueError("Cannot create a scene object for None. Please ensure you pass a instance of a supported class.") + raise SceneObjectNotRegisteredError("No scene object is registered for this data type: {} in this context: {}".format(itemtype, context)) - cls = _get_sceneobject_cls(item, **kwargs) PluginValidator.ensure_implementations(cls) return cls From 7ef3e4d702768db5c5ee1b137f4074aed96941e2 Mon Sep 17 00:00:00 2001 From: Licini Date: Thu, 15 May 2025 10:21:38 +0200 Subject: [PATCH 09/19] factory function instead of class --- src/compas/scene/scene.py | 2 +- src/compas/scene/sceneobject.py | 64 ++++++++++++++------------------ tests/compas/scene/test_scene.py | 12 +++--- 3 files changed, 35 insertions(+), 43 deletions(-) diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 395922678568..8f9a52d37cf4 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -119,7 +119,7 @@ def add(self, item, parent=None, **kwargs): del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error # Create a corresponding new scene object - sceneobject = SceneObjectFactory.create(item=item, context=self.context, scene=self, **kwargs) + sceneobject = SceneObjectFactory(item=item, context=self.context, scene=self, **kwargs) # Add the scene object and item to the data store self.objectstore[str(sceneobject.guid)] = sceneobject diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 42e909f5b872..a27e238b306d 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -21,47 +21,39 @@ from .descriptors.protocol import DescriptorProtocol -class SceneObjectFactory: - """Factory class for creating appropriate SceneObject instances based on input item type. +def SceneObjectFactory(item=None, scene=None, **kwargs): + """Create appropriate SceneObject instance based on item type. - This factory encapsulates the logic for selecting the right SceneObject subclass - for a given data item, making the creation process more explicit and easier to understand. + Parameters + ---------- + item : :class:`compas.data.Data` + The data item to create a scene object for. + **kwargs : dict + Additional keyword arguments to pass to the SceneObject constructor. + + Returns + ------- + :class:`compas.scene.SceneObject` + A SceneObject instance of the appropriate subclass for the given item. + + Raises + ------ + ValueError + If item is None. + SceneObjectNotRegisteredError + If no scene object is registered for the item type in the current context. """ + if item is None: + raise ValueError("Cannot create a scene object for None. Please ensure you pass an instance of a supported class.") - @staticmethod - def create(item=None, scene=None, **kwargs): - """Create appropriate SceneObject instance based on item type. - - Parameters - ---------- - item : :class:`compas.data.Data` - The data item to create a scene object for. - **kwargs : dict - Additional keyword arguments to pass to the SceneObject constructor. - - Returns - ------- - :class:`compas.scene.SceneObject` - A SceneObject instance of the appropriate subclass for the given item. - - Raises - ------ - ValueError - If item is None. - SceneObjectNotRegisteredError - If no scene object is registered for the item type in the current context. - """ - if item is None: - raise ValueError("Cannot create a scene object for None. Please ensure you pass an instance of a supported class.") - - if isinstance(item, SceneObject): - item._scene = scene - return item + if isinstance(item, SceneObject): + item._scene = scene + return item - sceneobject_cls = get_sceneobject_cls(item, **kwargs) + sceneobject_cls = get_sceneobject_cls(item, **kwargs) - # Create and return an instance of the appropriate scene object class - return sceneobject_cls(item=item, scene=scene, **kwargs) + # Create and return an instance of the appropriate scene object class + return sceneobject_cls(item=item, scene=scene, **kwargs) class SceneObject(Data): diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 31106876ca37..4dc73e5e1e18 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -47,29 +47,29 @@ def test_get_sceneobject_cls_with_orderly_registration(): register(FakeItem, FakeSceneObject, context="fake") register(FakeSubItem, FakeSubSceneObject, context="fake") item = FakeItem() - sceneobject = SceneObjectFactory.create(item, context="fake") + sceneobject = SceneObjectFactory(item=item, context="fake") assert isinstance(sceneobject, FakeSceneObject) item = FakeSubItem() - sceneobject = SceneObjectFactory.create(item, context="fake") + sceneobject = SceneObjectFactory(item=item, context="fake") assert isinstance(sceneobject, FakeSubSceneObject) def test_get_sceneobject_cls_with_out_of_order_registration(): register(FakeSubItem, FakeSubSceneObject, context="fake") register(FakeItem, FakeSceneObject, context="fake") item = FakeItem() - sceneobject = SceneObjectFactory.create(item, context="fake") + sceneobject = SceneObjectFactory(item=item, context="fake") assert isinstance(sceneobject, FakeSceneObject) item = FakeSubItem() - sceneobject = SceneObjectFactory.create(item, context="fake") + sceneobject = SceneObjectFactory(item=item, context="fake") assert isinstance(sceneobject, FakeSubSceneObject) def test_sceneobject_auto_context_discovery(mocker): register_fake_context() item = FakeItem() - sceneobject = SceneObjectFactory.create(item) + sceneobject = SceneObjectFactory(item=item) assert isinstance(sceneobject, FakeSceneObject) @@ -79,7 +79,7 @@ def test_sceneobject_auto_context_discovery_no_context(mocker): with pytest.raises(SceneObjectNotRegisteredError): item = FakeSubItem() - _ = SceneObjectFactory.create(item) + _ = SceneObjectFactory(item=item) def test_sceneobject_transform(): scene = Scene() From 55dcfaebb0e70d46a891337d24cc8742a3122221 Mon Sep 17 00:00:00 2001 From: Licini Date: Thu, 15 May 2025 10:46:25 +0200 Subject: [PATCH 10/19] docstring --- src/compas/scene/sceneobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index a27e238b306d..53d8989ebb41 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -278,6 +278,7 @@ def contrastcolor(self, color): self._contrastcolor = Color.coerce(color) def add(self, item, **kwargs): + # type: (compas.data.Data, dict) -> compas.scene.SceneObject """Add a child item to the scene object. Parameters @@ -292,7 +293,6 @@ def add(self, item, **kwargs): :class:`compas.scene.SceneObject` The added scene object. """ - # type: (compas.data.Data, dict) -> compas.scene.SceneObject return self.scene.add(item, parent=self, **kwargs) def remove(self): From 3e7a43d8420e94d6b920738b313b839a43ffc076 Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 16 May 2025 12:12:39 +0200 Subject: [PATCH 11/19] get_sceneobject_node --- src/compas/scene/scene.py | 36 ++++++++++++++++++++++++++++----- src/compas/scene/sceneobject.py | 2 +- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index 8f9a52d37cf4..d92ba64726ee 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -131,7 +131,7 @@ def add(self, item, parent=None, **kwargs): else: if not isinstance(parent, SceneObject): raise ValueError("Parent is not a SceneObject.", parent) - parent_node = self.tree.get_node_by_name(parent.guid) + parent_node = self.get_sceneobject_node(parent) if parent_node is None: raise ValueError("Parent is not part of the scene.", parent) @@ -148,13 +148,12 @@ def remove(self, sceneobject): The scene object to remove. """ # type: (SceneObject) -> None - guid = str(sceneobject.guid) - self.objectstore.pop(guid, None) - node = self.tree.get_node_by_name(guid) + node = self.get_sceneobject_node(sceneobject) if node: for descendant in node.descendants: self.objectstore.pop(descendant.name, None) self.tree.remove(node) + self.objectstore.pop(str(sceneobject.guid), None) def clear_context(self, guids=None): # type: (list | None) -> None @@ -265,7 +264,7 @@ def find_by_name(self, name): :class:`SceneObject` """ - return self.get_node_by_name(name=name) + return next((obj for obj in self.objects if obj.name == name), None) def find_by_itemtype(self, itemtype): # type: (type) -> SceneObject | None @@ -304,3 +303,30 @@ def find_all_by_itemtype(self, itemtype): if isinstance(obj.item, itemtype): sceneobjects.append(obj) return sceneobjects + + def get_sceneobject_node(self, sceneobject): + # type: (SceneObject) -> TreeNode + """Get the TreeNode that corresponds to a scene object. + + Parameters + ---------- + sceneobject : :class:`compas.scene.SceneObject` + + Returns + ------- + :class:`compas.datastructures.TreeNode` + + Raises + ------ + TypeError + If the scene object is not a :class:`compas.scene.SceneObject`. + ValueError + If the scene object is not part of this scene. + """ + + if not isinstance(sceneobject, SceneObject): + raise TypeError("SceneObject expected.", sceneobject) + if sceneobject.scene is not self: + raise ValueError("SceneObject not part of this scene.", sceneobject) + + return self.tree.get_node_by_name(sceneobject.guid) diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 53d8989ebb41..463a23742dab 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -186,7 +186,7 @@ def item(self): @property def node(self): # type: () -> compas.datastructures.TreeNode - return self.scene.tree.get_node_by_name(self.guid) + return self.scene.get_sceneobject_node(self) @property def is_root(self): From dd4344cead1f9ae0d0584849851cc7a709358bac Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 16 May 2025 12:31:55 +0200 Subject: [PATCH 12/19] more clean up and test coverage --- src/compas/scene/context.py | 10 ++--- tests/compas/datastructures/test_tree.py | 20 +++++++++ tests/compas/scene/test_scene.py | 56 ++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/src/compas/scene/context.py b/src/compas/scene/context.py index f814e12af4a2..bdb093953e14 100644 --- a/src/compas/scene/context.py +++ b/src/compas/scene/context.py @@ -119,7 +119,7 @@ def detect_current_context(): return None -def get_sceneobject_cls(item, context=None, **kwargs): +def get_sceneobject_cls(item, context=None, sceneobject_type=None): """Get the scene object class for a given item in the current context. If no context is provided, the current context is detected. If the exact item type is not registered, a closest match in its inheritance hierarchy is used. @@ -129,8 +129,8 @@ def get_sceneobject_cls(item, context=None, **kwargs): The item to get the scene object class for. context : Literal['Viewer', 'Rhino', 'Grasshopper', 'Blender'], optional The visualization context in which the pair should be registered. - **kwargs : dict - Additional keyword arguments. + sceneobject_type : :class:`~compas.scene.SceneObject`, optional + The scene object type to use. Raises ------ @@ -158,8 +158,8 @@ def get_sceneobject_cls(item, context=None, **kwargs): itemtype = type(item) cls = None - if "sceneobject_type" in kwargs: - cls = kwargs["sceneobject_type"] + if sceneobject_type is not None: + cls = sceneobject_type else: context = ITEM_SCENEOBJECT[context] diff --git a/tests/compas/datastructures/test_tree.py b/tests/compas/datastructures/test_tree.py index b04a42fa5663..d8d298598ae3 100644 --- a/tests/compas/datastructures/test_tree.py +++ b/tests/compas/datastructures/test_tree.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- import pytest import compas import json @@ -206,3 +207,22 @@ def key_mapper(node): assert graph2.has_edge(("root", "branch2")) assert graph2.has_edge(("branch2", "leaf2_1")) assert graph2.has_edge(("branch2", "leaf2_2")) + + +# ============================================================================= +# TreeNode Representation +# ============================================================================= + + +def test_treenode_representation(simple_tree): + def node_repr(node): + return node.name + " CUSTOM STRING" + print(simple_tree.get_hierarchy_string(node_repr=node_repr)) + + assert simple_tree.get_hierarchy_string(node_repr=node_repr) == """└── root CUSTOM STRING + ├── branch1 CUSTOM STRING + │ ├── leaf1_1 CUSTOM STRING + │ └── leaf1_2 CUSTOM STRING + └── branch2 CUSTOM STRING + ├── leaf2_1 CUSTOM STRING + └── leaf2_2 CUSTOM STRING""" diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 4dc73e5e1e18..8479797fa2d1 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -12,6 +12,7 @@ from compas.geometry import Box from compas.geometry import Frame from compas.geometry import Translation + from compas.scene import get_sceneobject_cls @pytest.fixture(autouse=True) def reset_sceneobjects_registration(): @@ -115,3 +116,58 @@ def test_scene_clear(): scene.clear(clear_context=False, clear_scene=True) assert len(scene.objects) == 0 + + def test_scene_context_validation(): + # Register the fake context first + register(FakeItem, FakeSceneObject, context="fake") + + scene = Scene(context="fake") + item = FakeItem() + + # This should work since the context matches + sceneobj = scene.add(item, context="fake") + assert isinstance(sceneobj, FakeSceneObject) + + # This should raise an exception since the context doesn't match + with pytest.raises(Exception) as excinfo: + scene.add(item, context="different") + assert "Object context should be the same as scene context" in str(excinfo.value) + + def test_get_sceneobject_cls_none_item(): + with pytest.raises(ValueError) as excinfo: + get_sceneobject_cls(None) + assert "Cannot create a scene object for None" in str(excinfo.value) + + def test_get_sceneobject_cls_auto_registration(): + # Clear the registration + context.ITEM_SCENEOBJECT.clear() + + # This should trigger auto-registration + item = FakeItem() + register(FakeItem, FakeSceneObject, context="fake") + cls = get_sceneobject_cls(item, context="fake") + assert cls == FakeSceneObject + + def test_get_sceneobject_cls_inheritance(): + # Register base class + register(FakeItem, FakeSceneObject, context="fake") + + # Test that subclass uses base class's scene object + item = FakeSubItem() + cls = get_sceneobject_cls(item, context="fake") + assert cls == FakeSceneObject + + def test_get_sceneobject_cls_custom_type(): + item = FakeItem() + cls = get_sceneobject_cls(item, context="fake", sceneobject_type=FakeSubSceneObject) + assert cls == FakeSubSceneObject + + def test_get_sceneobject_cls_no_registration(): + # Clear the registration + context.ITEM_SCENEOBJECT.clear() + + # Try to get scene object for unregistered item + item = FakeItem() + with pytest.raises(SceneObjectNotRegisteredError) as excinfo: + get_sceneobject_cls(item, context="fake") + assert "No scene object is registered for this data type" in str(excinfo.value) From 981464d027646bf25e3b3a21c34ab0c8af2ba3d8 Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 16 May 2025 12:33:32 +0200 Subject: [PATCH 13/19] lint --- tests/compas/datastructures/test_tree.py | 6 +++++- tests/compas/scene/test_scene.py | 12 ++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/compas/datastructures/test_tree.py b/tests/compas/datastructures/test_tree.py index d8d298598ae3..3065436c4707 100644 --- a/tests/compas/datastructures/test_tree.py +++ b/tests/compas/datastructures/test_tree.py @@ -217,12 +217,16 @@ def key_mapper(node): def test_treenode_representation(simple_tree): def node_repr(node): return node.name + " CUSTOM STRING" + print(simple_tree.get_hierarchy_string(node_repr=node_repr)) - assert simple_tree.get_hierarchy_string(node_repr=node_repr) == """└── root CUSTOM STRING + assert ( + simple_tree.get_hierarchy_string(node_repr=node_repr) + == """└── root CUSTOM STRING ├── branch1 CUSTOM STRING │ ├── leaf1_1 CUSTOM STRING │ └── leaf1_2 CUSTOM STRING └── branch2 CUSTOM STRING ├── leaf2_1 CUSTOM STRING └── leaf2_2 CUSTOM STRING""" + ) diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 8479797fa2d1..ef61d6b3ccca 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -120,14 +120,14 @@ def test_scene_clear(): def test_scene_context_validation(): # Register the fake context first register(FakeItem, FakeSceneObject, context="fake") - + scene = Scene(context="fake") item = FakeItem() - + # This should work since the context matches sceneobj = scene.add(item, context="fake") assert isinstance(sceneobj, FakeSceneObject) - + # This should raise an exception since the context doesn't match with pytest.raises(Exception) as excinfo: scene.add(item, context="different") @@ -141,7 +141,7 @@ def test_get_sceneobject_cls_none_item(): def test_get_sceneobject_cls_auto_registration(): # Clear the registration context.ITEM_SCENEOBJECT.clear() - + # This should trigger auto-registration item = FakeItem() register(FakeItem, FakeSceneObject, context="fake") @@ -151,7 +151,7 @@ def test_get_sceneobject_cls_auto_registration(): def test_get_sceneobject_cls_inheritance(): # Register base class register(FakeItem, FakeSceneObject, context="fake") - + # Test that subclass uses base class's scene object item = FakeSubItem() cls = get_sceneobject_cls(item, context="fake") @@ -165,7 +165,7 @@ def test_get_sceneobject_cls_custom_type(): def test_get_sceneobject_cls_no_registration(): # Clear the registration context.ITEM_SCENEOBJECT.clear() - + # Try to get scene object for unregistered item item = FakeItem() with pytest.raises(SceneObjectNotRegisteredError) as excinfo: From 85c33002f82a38dfa92abb677e44e8ccb4831ae3 Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 16 May 2025 12:42:38 +0200 Subject: [PATCH 14/19] add coverage --- tests/compas/scene/test_scene.py | 36 ++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index ef61d6b3ccca..32b958de5af2 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -171,3 +171,39 @@ def test_get_sceneobject_cls_no_registration(): with pytest.raises(SceneObjectNotRegisteredError) as excinfo: get_sceneobject_cls(item, context="fake") assert "No scene object is registered for this data type" in str(excinfo.value) + + def test_scene_representation(): + # Create a scene with a hierarchy of objects + scene = Scene(context="fake") + register(FakeItem, FakeSceneObject, context="fake") + + # Create items and add them to the scene + root_item = FakeItem() + child1_item = FakeItem() + child2_item = FakeItem() + grandchild1_item = FakeItem() + grandchild2_item = FakeItem() + + # Add objects to create a hierarchy + root = scene.add(root_item) + child1 = scene.add(child1_item, parent=root) + scene.add(child2_item, parent=root) + scene.add(grandchild1_item, parent=child1) + scene.add(grandchild2_item, parent=child1) + + # Get the string representation + scene_repr = str(scene) + + # The representation should show the hierarchy with scene objects + expected_lines = [ + "└── Scene", + " └── ", + " ├── ", + " │ ├── ", + " │ └── ", + " └── ", + ] + + # Compare each line + for expected, actual in zip(expected_lines, scene_repr.split("\n")): + assert expected == actual From 44163d399ddef914970880f9283d11d88498170d Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 16 May 2025 12:56:30 +0200 Subject: [PATCH 15/19] more tests --- src/compas/scene/sceneobject.py | 2 +- tests/compas/scene/test_scene.py | 165 ++++++++++++++++++++++++++++++- 2 files changed, 164 insertions(+), 3 deletions(-) diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 463a23742dab..90047994d939 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -181,7 +181,7 @@ def scene(self): @property def item(self): # type: () -> compas.data.Data - return self.scene.items[self._item] + return self.scene.datastore[self._item] @property def node(self): diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 32b958de5af2..79ea42baeedb 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -13,6 +13,7 @@ from compas.geometry import Frame from compas.geometry import Translation from compas.scene import get_sceneobject_cls + from compas.datastructures import Tree @pytest.fixture(autouse=True) def reset_sceneobjects_registration(): @@ -75,8 +76,8 @@ def test_sceneobject_auto_context_discovery(mocker): assert isinstance(sceneobject, FakeSceneObject) def test_sceneobject_auto_context_discovery_no_context(mocker): - mocker.patch("compas.scene.context.compas.is_grasshopper", return_value=False) - mocker.patch("compas.scene.context.compas.is_rhino", return_value=False) + mocker.patch("compas.scene.scene.detect_current_context", return_value=False) + mocker.patch("compas.scene.scene.detect_current_context", return_value=False) with pytest.raises(SceneObjectNotRegisteredError): item = FakeSubItem() @@ -207,3 +208,163 @@ def test_scene_representation(): # Compare each line for expected, actual in zip(expected_lines, scene_repr.split("\n")): assert expected == actual + + def test_scene_initialization(mocker): + # Mock context detection at the correct import path + mocker.patch("compas.scene.scene.detect_current_context", return_value="fake") + + # Test default initialization + scene = Scene() + assert scene.context == "fake" + assert scene.datastore == {} + assert scene.objectstore == {} + assert scene.tree is not None + assert scene.tree.root is not None + assert scene.tree.root.name == scene.name + + # Test initialization with custom parameters + custom_tree = Tree() + custom_datastore = {"test": "data"} + custom_objectstore = {"test": "object"} + scene = Scene(context="test", tree=custom_tree, datastore=custom_datastore, objectstore=custom_objectstore) + assert scene.context == "test" + assert scene.tree == custom_tree + assert scene.datastore == custom_datastore + assert scene.objectstore == custom_objectstore + + def test_scene_data(): + scene = Scene() + data = scene.__data__ + assert "name" in data + assert "attributes" in data + assert "datastore" in data + assert "objectstore" in data + assert "tree" in data + + def test_scene_items(): + scene = Scene(context="fake") + register(FakeItem, FakeSceneObject, context="fake") + + item1 = FakeItem() + item2 = FakeItem() + scene.add(item1) + scene.add(item2) + + items = scene.items + assert len(items) == 2 + assert item1 in items + assert item2 in items + + def test_scene_context_objects(mocker): + mocker.patch("compas.scene.scene.detect_current_context", return_value="fake") + scene = Scene(context="fake") + register(FakeItem, FakeSceneObject, context="fake") + + item = FakeItem() + sceneobj = scene.add(item) + + # Mock the _guids attribute to return test guids + sceneobj._guids = ["guid1", "guid2"] + + context_objects = scene.context_objects + assert len(context_objects) == 2 + assert "guid1" in context_objects + assert "guid2" in context_objects + + def test_scene_find_by_name(): + scene = Scene(context="fake") + register(FakeItem, FakeSceneObject, context="fake") + + item = FakeItem() + sceneobj = scene.add(item) + sceneobj.name = "test_object" + + found = scene.find_by_name("test_object") + assert found == sceneobj + + not_found = scene.find_by_name("nonexistent") + assert not_found is None + + def test_scene_find_by_itemtype(mocker): + mocker.patch("compas.scene.scene.detect_current_context", return_value="fake") + scene = Scene(context="fake") + register(FakeItem, FakeSceneObject, context="fake") + register(FakeSubItem, FakeSubSceneObject, context="fake") + + # Create items and add them to the scene + item1 = FakeItem() + item2 = FakeSubItem() + sceneobj1 = scene.add(item1) + sceneobj2 = scene.add(item2) + + # Ensure the datastore is properly set up + scene.datastore[str(item1.guid)] = item1 + scene.datastore[str(item2.guid)] = item2 + + # Find objects by type + found = scene.find_by_itemtype(FakeItem) + assert found is not None + assert found._item == str(item1.guid) + + found = scene.find_by_itemtype(FakeSubItem) + assert found is not None + assert found._item == str(item2.guid) + + not_found = scene.find_by_itemtype(str) # type that doesn't exist in scene + assert not_found is None + + def test_scene_find_all_by_itemtype(mocker): + mocker.patch("compas.scene.scene.detect_current_context", return_value="fake") + scene = Scene(context="fake") + register(FakeItem, FakeSceneObject, context="fake") + register(FakeSubItem, FakeSubSceneObject, context="fake") + + # Create items and add them to the scene + item1 = FakeItem() + item2 = FakeSubItem() + item3 = FakeItem() + sceneobj1 = scene.add(item1) + sceneobj2 = scene.add(item2) + sceneobj3 = scene.add(item3) + + # Ensure the datastore is properly set up + scene.datastore[str(item1.guid)] = item1 + scene.datastore[str(item2.guid)] = item2 + scene.datastore[str(item3.guid)] = item3 + + # Find all objects by type + found = scene.find_all_by_itemtype(FakeItem) + assert len(found) == 3 + assert all(obj._item in [str(item1.guid), str(item2.guid), str(item3.guid)] for obj in found) + + found = scene.find_all_by_itemtype(FakeSubItem) + assert len(found) == 1 + assert all(obj._item == str(item2.guid) for obj in found) + + not_found = scene.find_all_by_itemtype(str) # type that doesn't exist in scene + assert len(not_found) == 0 + + def test_scene_get_sceneobject_node(): + scene = Scene(context="fake") + register(FakeItem, FakeSceneObject, context="fake") + + item = FakeItem() + sceneobj = scene.add(item) + + # Test successful case + node = scene.get_sceneobject_node(sceneobj) + assert node is not None + assert node.name == str(sceneobj.guid) + + # Test TypeError for non-SceneObject + with pytest.raises(TypeError) as excinfo: + scene.get_sceneobject_node("not a scene object") + assert "SceneObject expected" in str(excinfo.value) + + # Test ValueError for SceneObject from different scene + other_scene = Scene(context="fake") + other_item = FakeItem() + other_sceneobj = other_scene.add(other_item) + with pytest.raises(ValueError) as excinfo: + scene.get_sceneobject_node(other_sceneobj) + assert "SceneObject not part of this scene" in str(excinfo.value) From ab5425a8a16432d9217b2a0d28e3a8f97a38a667 Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 16 May 2025 12:58:09 +0200 Subject: [PATCH 16/19] lint --- tests/compas/scene/test_scene.py | 43 +++++++++++++------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 79ea42baeedb..3d05ff39a7fe 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -212,7 +212,7 @@ def test_scene_representation(): def test_scene_initialization(mocker): # Mock context detection at the correct import path mocker.patch("compas.scene.scene.detect_current_context", return_value="fake") - + # Test default initialization scene = Scene() assert scene.context == "fake" @@ -259,13 +259,13 @@ def test_scene_context_objects(mocker): mocker.patch("compas.scene.scene.detect_current_context", return_value="fake") scene = Scene(context="fake") register(FakeItem, FakeSceneObject, context="fake") - + item = FakeItem() sceneobj = scene.add(item) - + # Mock the _guids attribute to return test guids sceneobj._guids = ["guid1", "guid2"] - + context_objects = scene.context_objects assert len(context_objects) == 2 assert "guid1" in context_objects @@ -290,26 +290,22 @@ def test_scene_find_by_itemtype(mocker): scene = Scene(context="fake") register(FakeItem, FakeSceneObject, context="fake") register(FakeSubItem, FakeSubSceneObject, context="fake") - + # Create items and add them to the scene item1 = FakeItem() item2 = FakeSubItem() - sceneobj1 = scene.add(item1) - sceneobj2 = scene.add(item2) - - # Ensure the datastore is properly set up - scene.datastore[str(item1.guid)] = item1 - scene.datastore[str(item2.guid)] = item2 - + scene.add(item1) + scene.add(item2) + # Find objects by type found = scene.find_by_itemtype(FakeItem) assert found is not None assert found._item == str(item1.guid) - + found = scene.find_by_itemtype(FakeSubItem) assert found is not None assert found._item == str(item2.guid) - + not_found = scene.find_by_itemtype(str) # type that doesn't exist in scene assert not_found is None @@ -318,29 +314,24 @@ def test_scene_find_all_by_itemtype(mocker): scene = Scene(context="fake") register(FakeItem, FakeSceneObject, context="fake") register(FakeSubItem, FakeSubSceneObject, context="fake") - + # Create items and add them to the scene item1 = FakeItem() item2 = FakeSubItem() item3 = FakeItem() - sceneobj1 = scene.add(item1) - sceneobj2 = scene.add(item2) - sceneobj3 = scene.add(item3) - - # Ensure the datastore is properly set up - scene.datastore[str(item1.guid)] = item1 - scene.datastore[str(item2.guid)] = item2 - scene.datastore[str(item3.guid)] = item3 - + scene.add(item1) + scene.add(item2) + scene.add(item3) + # Find all objects by type found = scene.find_all_by_itemtype(FakeItem) assert len(found) == 3 assert all(obj._item in [str(item1.guid), str(item2.guid), str(item3.guid)] for obj in found) - + found = scene.find_all_by_itemtype(FakeSubItem) assert len(found) == 1 assert all(obj._item == str(item2.guid) for obj in found) - + not_found = scene.find_all_by_itemtype(str) # type that doesn't exist in scene assert len(not_found) == 0 From 38a7d633e85593a6dd320cf9f60d74ded7288c74 Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 16 May 2025 15:34:51 +0200 Subject: [PATCH 17/19] changelog --- CHANGELOG.md | 7 +++++++ tests/compas/scene/test_scene.py | 1 + 2 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3495d78de47b..31a692bf9ddd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * Added `inheritance` field to `__jsondump__` of `compas.datastructures.Datastructure` to allow for deserialization to closest available superclass of custom datastructures. +* Added `compas.scene.Scene.get_sceneobject_node` to get the TreeNode that corresponds to a scene object. +* Added `compas.scene.SceneObjectFactory` method to create appropriate scene objects from data. ### Changed +* Changed `compas.scene.Scene` to use underlying `datastore`, `objectstore` and `tree` attributes for more transparent serialization and deserialization processes. +* Changed `compas.scene.SceneObject` to use object `guid` to retrieve the corresponding TreeNode from the scene tree, and use item `guid` to retrieve the corresponding data item from the scene datastore. + ### Removed +* Removed `compas.scene.SceneObject.__new__` method, explicitly use `compas.scene.SceneObjectFactory` instead. + ## [2.11.0] 2025-04-22 diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 3d05ff39a7fe..24a3ebab40ab 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- import compas if not compas.IPY: From ef4def8e15ec2ae14a494f8f7f1ba0e318f2daef Mon Sep 17 00:00:00 2001 From: Licini Date: Fri, 16 May 2025 16:39:11 +0200 Subject: [PATCH 18/19] clean up kwargs --- src/compas/scene/context.py | 20 ++++++++------------ src/compas/scene/sceneobject.py | 10 +++++++--- tests/compas/scene/test_scene.py | 5 ----- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/compas/scene/context.py b/src/compas/scene/context.py index bdb093953e14..1ca86be66739 100644 --- a/src/compas/scene/context.py +++ b/src/compas/scene/context.py @@ -119,7 +119,7 @@ def detect_current_context(): return None -def get_sceneobject_cls(item, context=None, sceneobject_type=None): +def get_sceneobject_cls(item, context=None): """Get the scene object class for a given item in the current context. If no context is provided, the current context is detected. If the exact item type is not registered, a closest match in its inheritance hierarchy is used. @@ -129,8 +129,6 @@ def get_sceneobject_cls(item, context=None, sceneobject_type=None): The item to get the scene object class for. context : Literal['Viewer', 'Rhino', 'Grasshopper', 'Blender'], optional The visualization context in which the pair should be registered. - sceneobject_type : :class:`~compas.scene.SceneObject`, optional - The scene object type to use. Raises ------ @@ -156,17 +154,15 @@ def get_sceneobject_cls(item, context=None, sceneobject_type=None): context = detect_current_context() itemtype = type(item) - cls = None - if sceneobject_type is not None: - cls = sceneobject_type - else: - context = ITEM_SCENEOBJECT[context] + context = ITEM_SCENEOBJECT[context] + + cls = None - for inheritancetype in inspect.getmro(itemtype): - cls = context.get(inheritancetype, None) - if cls is not None: - break + for inheritancetype in inspect.getmro(itemtype): + cls = context.get(inheritancetype, None) + if cls is not None: + break if cls is None: raise SceneObjectNotRegisteredError("No scene object is registered for this data type: {} in this context: {}".format(itemtype, context)) diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 90047994d939..59754915c156 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -21,13 +21,17 @@ from .descriptors.protocol import DescriptorProtocol -def SceneObjectFactory(item=None, scene=None, **kwargs): +def SceneObjectFactory(item=None, scene=None, context=None, **kwargs): """Create appropriate SceneObject instance based on item type. Parameters ---------- item : :class:`compas.data.Data` The data item to create a scene object for. + scene : :class:`compas.scene.Scene`, optional + The scene in which the scene object is created. + context : str, optional + The context in which the scene object is created. **kwargs : dict Additional keyword arguments to pass to the SceneObject constructor. @@ -50,10 +54,10 @@ def SceneObjectFactory(item=None, scene=None, **kwargs): item._scene = scene return item - sceneobject_cls = get_sceneobject_cls(item, **kwargs) + sceneobject_cls = get_sceneobject_cls(item, context=context) # Create and return an instance of the appropriate scene object class - return sceneobject_cls(item=item, scene=scene, **kwargs) + return sceneobject_cls(item=item, scene=scene, context=context, **kwargs) class SceneObject(Data): diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index 24a3ebab40ab..fba06ef64058 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -159,11 +159,6 @@ def test_get_sceneobject_cls_inheritance(): cls = get_sceneobject_cls(item, context="fake") assert cls == FakeSceneObject - def test_get_sceneobject_cls_custom_type(): - item = FakeItem() - cls = get_sceneobject_cls(item, context="fake", sceneobject_type=FakeSubSceneObject) - assert cls == FakeSubSceneObject - def test_get_sceneobject_cls_no_registration(): # Clear the registration context.ITEM_SCENEOBJECT.clear() From 01cf1afc8254ef1b49b99a271141ee5af9109c3f Mon Sep 17 00:00:00 2001 From: Licini Date: Mon, 26 May 2025 17:36:06 +0200 Subject: [PATCH 19/19] naming pattern, type hints and changelog --- CHANGELOG.md | 5 +++-- src/compas/scene/__init__.py | 4 ++-- src/compas/scene/scene.py | 12 ++++++------ src/compas/scene/sceneobject.py | 6 ++++-- .../components/Compas_ToRhinoGeometry/code.py | 4 ++-- .../Compas_ToRhinoGeometry/code.py | 4 ++-- tests/compas/scene/test_scene.py | 14 +++++++------- 7 files changed, 26 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31a692bf9ddd..a6490d0720c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Added `inheritance` field to `__jsondump__` of `compas.datastructures.Datastructure` to allow for deserialization to closest available superclass of custom datastructures. * Added `compas.scene.Scene.get_sceneobject_node` to get the TreeNode that corresponds to a scene object. -* Added `compas.scene.SceneObjectFactory` method to create appropriate scene objects from data. +* Added `compas.scene.sceneobject_factory` method to create appropriate scene objects from data. ### Changed @@ -20,7 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed -* Removed `compas.scene.SceneObject.__new__` method, explicitly use `compas.scene.SceneObjectFactory` instead. +* Removed `compas.scene.SceneObject.__new__` method, explicitly use `compas.scene.sceneobject_factory` instead. +* Removed `frame` kwarg from `compas.scene.SceneObject` constructor, since it is now computed from the `worldtransformation` attribute. ## [2.11.0] 2025-04-22 diff --git a/src/compas/scene/__init__.py b/src/compas/scene/__init__.py index 0c5285c8f7de..2bbce37b507f 100644 --- a/src/compas/scene/__init__.py +++ b/src/compas/scene/__init__.py @@ -10,7 +10,7 @@ from .exceptions import SceneObjectNotRegisteredError from .sceneobject import SceneObject -from .sceneobject import SceneObjectFactory +from .sceneobject import sceneobject_factory from .meshobject import MeshObject from .graphobject import GraphObject from .geometryobject import GeometryObject @@ -44,7 +44,7 @@ def register_scene_objects_base(): __all__ = [ "SceneObjectNotRegisteredError", "SceneObject", - "SceneObjectFactory", + "sceneobject_factory", "MeshObject", "GraphObject", "GeometryObject", diff --git a/src/compas/scene/scene.py b/src/compas/scene/scene.py index d92ba64726ee..11b0d060be7f 100644 --- a/src/compas/scene/scene.py +++ b/src/compas/scene/scene.py @@ -10,7 +10,7 @@ from .context import clear from .context import detect_current_context from .sceneobject import SceneObject -from .sceneobject import SceneObjectFactory +from .sceneobject import sceneobject_factory class Scene(Datastructure): @@ -88,7 +88,7 @@ def objects(self): @property def context_objects(self): - # type: () -> list[SceneObject] + # type: () -> list guids = [] for obj in self.objects: guids += obj.guids @@ -119,7 +119,7 @@ def add(self, item, parent=None, **kwargs): del kwargs["context"] # otherwist the SceneObject receives "context" twice, which results in an error # Create a corresponding new scene object - sceneobject = SceneObjectFactory(item=item, context=self.context, scene=self, **kwargs) + sceneobject = sceneobject_factory(item=item, context=self.context, scene=self, **kwargs) # Add the scene object and item to the data store self.objectstore[str(sceneobject.guid)] = sceneobject @@ -251,7 +251,7 @@ def redraw(self): self.draw() def find_by_name(self, name): - # type: (str) -> SceneObject + # type: (str) -> SceneObject | None """Find the first scene object with the given name. Parameters @@ -261,7 +261,7 @@ def find_by_name(self, name): Returns ------- - :class:`SceneObject` + :class:`SceneObject` | None """ return next((obj for obj in self.objects if obj.name == name), None) @@ -277,7 +277,7 @@ def find_by_itemtype(self, itemtype): Returns ------- - :class:`SceneObject` or None + :class:`SceneObject` | None """ for obj in self.objects: diff --git a/src/compas/scene/sceneobject.py b/src/compas/scene/sceneobject.py index 59754915c156..a423eec9f1f0 100644 --- a/src/compas/scene/sceneobject.py +++ b/src/compas/scene/sceneobject.py @@ -21,7 +21,7 @@ from .descriptors.protocol import DescriptorProtocol -def SceneObjectFactory(item=None, scene=None, context=None, **kwargs): +def sceneobject_factory(item=None, scene=None, context=None, **kwargs): """Create appropriate SceneObject instance based on item type. Parameters @@ -190,7 +190,9 @@ def item(self): @property def node(self): # type: () -> compas.datastructures.TreeNode - return self.scene.get_sceneobject_node(self) + if self._node is None: + self._node = self.scene.get_sceneobject_node(self) + return self._node @property def is_root(self): diff --git a/src/compas_ghpython/components/Compas_ToRhinoGeometry/code.py b/src/compas_ghpython/components/Compas_ToRhinoGeometry/code.py index fcc03a1ca0b0..d85be514217f 100644 --- a/src/compas_ghpython/components/Compas_ToRhinoGeometry/code.py +++ b/src/compas_ghpython/components/Compas_ToRhinoGeometry/code.py @@ -4,7 +4,7 @@ from ghpythonlib.componentbase import executingcomponent as component -from compas.scene import SceneObject +from compas.scene import sceneobject_factory class CompasToRhinoGeometry(component): @@ -12,4 +12,4 @@ def RunScript(self, cg): if not cg: return None - return SceneObject(item=cg).draw() + return sceneobject_factory(item=cg).draw() diff --git a/src/compas_ghpython/components_cpython/Compas_ToRhinoGeometry/code.py b/src/compas_ghpython/components_cpython/Compas_ToRhinoGeometry/code.py index 93a00e65ad50..a05791018916 100644 --- a/src/compas_ghpython/components_cpython/Compas_ToRhinoGeometry/code.py +++ b/src/compas_ghpython/components_cpython/Compas_ToRhinoGeometry/code.py @@ -7,7 +7,7 @@ import Grasshopper -from compas.scene import SceneObject +from compas.scene import sceneobject_factory class CompasToRhinoGeometry(Grasshopper.Kernel.GH_ScriptInstance): @@ -15,4 +15,4 @@ def RunScript(self, cg: Any): if not cg: return None - return SceneObject(item=cg).draw() + return sceneobject_factory(item=cg).draw() diff --git a/tests/compas/scene/test_scene.py b/tests/compas/scene/test_scene.py index fba06ef64058..ea3f89a87ec4 100644 --- a/tests/compas/scene/test_scene.py +++ b/tests/compas/scene/test_scene.py @@ -7,7 +7,7 @@ from compas.scene import register from compas.scene import Scene from compas.scene import SceneObject - from compas.scene import SceneObjectFactory + from compas.scene import sceneobject_factory from compas.scene import SceneObjectNotRegisteredError from compas.data import Data from compas.geometry import Box @@ -50,29 +50,29 @@ def test_get_sceneobject_cls_with_orderly_registration(): register(FakeItem, FakeSceneObject, context="fake") register(FakeSubItem, FakeSubSceneObject, context="fake") item = FakeItem() - sceneobject = SceneObjectFactory(item=item, context="fake") + sceneobject = sceneobject_factory(item=item, context="fake") assert isinstance(sceneobject, FakeSceneObject) item = FakeSubItem() - sceneobject = SceneObjectFactory(item=item, context="fake") + sceneobject = sceneobject_factory(item=item, context="fake") assert isinstance(sceneobject, FakeSubSceneObject) def test_get_sceneobject_cls_with_out_of_order_registration(): register(FakeSubItem, FakeSubSceneObject, context="fake") register(FakeItem, FakeSceneObject, context="fake") item = FakeItem() - sceneobject = SceneObjectFactory(item=item, context="fake") + sceneobject = sceneobject_factory(item=item, context="fake") assert isinstance(sceneobject, FakeSceneObject) item = FakeSubItem() - sceneobject = SceneObjectFactory(item=item, context="fake") + sceneobject = sceneobject_factory(item=item, context="fake") assert isinstance(sceneobject, FakeSubSceneObject) def test_sceneobject_auto_context_discovery(mocker): register_fake_context() item = FakeItem() - sceneobject = SceneObjectFactory(item=item) + sceneobject = sceneobject_factory(item=item) assert isinstance(sceneobject, FakeSceneObject) @@ -82,7 +82,7 @@ def test_sceneobject_auto_context_discovery_no_context(mocker): with pytest.raises(SceneObjectNotRegisteredError): item = FakeSubItem() - _ = SceneObjectFactory(item=item) + _ = sceneobject_factory(item=item) def test_sceneobject_transform(): scene = Scene()