Scene Data Store #1461

Licini · 2025-05-14T11:03:16Z

Hey guys a rather large PR, there are several components to this, let me explain:

1. Scene data store

Previously we inherent Scene from Tree and SceneObject from TreeNode. It was nice but the serialization was not very straightforward, because we don't want to serialize Data item directly under SceneObject since they might be reused. So we had to customize the serialization and deserialization to only store guid of item under SceneObject, then group all the distinct items as a list under scene.

This PR basically makes the idea more explicit by reflecting this mechanism directly as the attributes of Scene. Under Scene, we have 3 main aspects: datastore, objectstore and tree. The datastore is a bucket of unique Data items, the objectstore is a bucket of SceneObjects, both keyed by their guids. And tree stores hierarchy of the scene objects, but nothing more.

For SceneObject, instead maintaining a live link to Data item, we only actually store guid of that item and use the datastore of the scene to retrieve it (with minimal overhead).

The benefit is a much more transparent serialization and deserialization process: now both Scene and SceneObject can be json dumped/loaded without any customization using our default encoder/decoder. The structure of two classes directly reflect the serialized data with almost no change.

This also makes it easier for further extension of Scene for example if you want to use a datastore that contains very large geometry but with lazy loading, you can pass in a custom datastore yourself when creating the scene, using dict-like object which can be queried with a guid.

For example a minimal serialized scene can look like this now:

{
    "data": {
        "attributes": {},
        "datastore": {
            "07b131a8-6edc-407a-8944-174a274fec5b": {
                "data": {
                    # ...
                },
                "dtype": "compas.geometry/Box",
                "guid": "07b131a8-6edc-407a-8944-174a274fec5b"
            }
        },
        "name": "Scene",
        "objectstore": {
            "22fd4e9c-66fe-481a-9392-c07c7fea2522": {
                "data": {
                    "item": "07b131a8-6edc-407a-8944-174a274fec5b",
                    # ...
                },
                "dtype": "compas.scene/GeometryObject",
                "guid": "22fd4e9c-66fe-481a-9392-c07c7fea2522",
                "name": "box1"
            },
            "bea576ac-641e-400b-9083-0ed018003af0": {
                "data": {
                    "item": "07b131a8-6edc-407a-8944-174a274fec5b",
                    # ...
                },
                "dtype": "compas.scene/GeometryObject",
                "guid": "bea576ac-641e-400b-9083-0ed018003af0",
                "name": "box2"
            }
        },
        "tree": {
            "data": {
                "attributes": {},
                "root": {
                    "children": [
                        {
                            "children": [
                                {
                                    "name": "bea576ac-641e-400b-9083-0ed018003af0"
                                }
                            ],
                            "name": "22fd4e9c-66fe-481a-9392-c07c7fea2522"
                        }
                    ],
                    "name": "Scene"
                }
            },
            "dtype": "compas.datastructures/Tree",
            "guid": "ef7305fd-cabc-45c2-a66e-e3cdc367ec25",
            "inheritance": []
        }
    },
    "dtype": "compas.scene/Scene",
    "guid": "ac6b3bc4-bda2-4c5c-9d0e-8496d3590cb5",
    "inheritance": []
}

2. SceneObjectFactory

We have been using __new__ for automatically choosing appropriate SceneObject class. It has been working well most of time. But it becomes annoying if one wants to initialize a custom SceneObject class like Group, where I just want a straightforward initiation without triggering auto class detection. @tomvanmele has been advocating for more explicit Factory method instead __new__, to make the constructor behavior more predictable and more type-hint friendly. Which I quite agree.

This does not change the external APIs, users still use Scene.add the same way, which internally calls the factory method now instead of the class constructor.

3. Others

I added more tests on Scene to make sure it behaves correctly. In summary, although there are a big internal re-organization in Scene and SceneObject, the user-facing APIs aren't changed, there also shouldn't be any changes needed for all the extended SceneObjects. I will drop some in-line comments below

src/compas/scene/scene.py

src/compas/scene/sceneobject.py

codecov · 2025-05-15T08:48:35Z

Codecov Report

Attention: Patch coverage is 90.55118% with 12 lines in your changes missing coverage. Please review.

Project coverage is 61.93%. Comparing base (c1ac434) to head (01cf1af).
Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
src/compas/scene/sceneobject.py	83.01%	9 Missing ⚠️
src/compas/scene/scene.py	96.00%	2 Missing ⚠️
src/compas/datastructures/tree/tree.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1461      +/-   ##
==========================================
- Coverage   61.94%   61.93%   -0.01%     
==========================================
  Files         208      208              
  Lines       22365    22401      +36     
==========================================
+ Hits        13853    13875      +22     
- Misses       8512     8526      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Licini · 2025-05-16T14:57:14Z

src/compas/scene/context.py

Consolidating _get_sceneobject_cls and get_sceneobject_cls into one function. Also cleaned up the kwargs. I removed sceneobject_type parameter because now you can call constructor directly with __new__ removed.

Licini · 2025-05-16T14:59:49Z

src/compas/scene/group.py

-
-    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


These customizations are no longer needed anymore, thanks to the more straight forward serialization/deserialization

Licini · 2025-05-16T15:03:17Z

src/compas/scene/scene.py

    @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()),
+            "attributes": self.attributes,
+            "datastore": self.datastore,
+            "objectstore": self.objectstore,
+            "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


We directly serialize datastore, objectstore and tree here. And during deserialization we can read them as they are without using a custom __from__data__ method.

Licini · 2025-05-16T15:04:24Z

src/compas/scene/scene.py

-        # type: (str, str | None) -> None
-        super(Scene, self).__init__(name=name)
-        super(Scene, self).add(TreeNode(name="ROOT"))
+    def __init__(self, context=None, datastore=None, objectstore=None, tree=None, **kwargs):


Users can passing their own customs stores and tree, if they know what they are doing.

Licini · 2025-05-16T15:06:26Z

src/compas/scene/sceneobject.py

    def item(self):
        # type: () -> compas.data.Data
-        return self._item
+        return self.scene.datastore[self._item]


item now becomes a getter that retrieve the live object from datastore, what we actually store as _item is the guid

Licini · 2025-05-16T15:07:48Z

src/compas/scene/sceneobject.py

+    @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


"shortcut" properties that allows us to still work with SceneObject like a TreeNode

src/compas/scene/scene.py

src/compas/scene/sceneobject.py

tomvanmele · 2025-05-25T13:54:41Z

@Licini shall we try to finish this?

Licini · 2025-05-26T15:38:12Z

src/compas_ghpython/components/Compas_ToRhinoGeometry/code.py


-from compas.scene import SceneObject
+from compas.scene import sceneobject_factory


 class CompasToRhinoGeometry(component):
    def RunScript(self, cg):
        if not cg:
            return None

-        return SceneObject(item=cg).draw()
+        return sceneobject_factory(item=cg).draw()


@gonzalocasas @chenkasirer Please note here, with the factory pattern, you need call factory function in GH components instead of constructor

This is a breaking change that affects a lot of external code. It needs a major release (compas 3!). Also, I don't like the method name sceneobject_factory, it feels old, could we add the factory method as a class/static method of SceneObject, like SceneObject.create()? the word factory on its own sounds too much java-like to my taste :P

This is a breaking change that affects a lot of external code. It needs a major release (compas 3!). Also, I don't like the method name sceneobject_factory, it feels old, could we add the factory method as a class/static method of SceneObject, like SceneObject.create()? the word factory on its own sounds too much java-like to my taste :P

Ok 🤣 I had impression that these are the only places SceneOjbect constructor are called. But sure SceneObject.create() is a good idea to me too. Regarding the full PR, I have been having more discussions @tomvanmele on potential issues and corresponding reworks for data store, so I will close this one for now and re-submit another one that only concerns the serialization part. We can deal with __new__ in the future in compas 3

Licini · 2025-05-26T15:39:22Z

@Licini shall we try to finish this?

Done with the requested changes

Licini added 5 commits April 2, 2025 09:38

add scene object factory

58571a0

add node_repr option to tree

eff2b2e

use "data store" for Scene

3d5e493

Merge branch 'main' into scene

f07bdbf

finish the work, update tests

10e4225

Licini changed the title ~~Scene~~ Scene Data Store May 14, 2025

Licini added 3 commits May 14, 2025 13:09

put back setter

e8b5fab

objectstore

82fa199

fixes

b730a4d

tomvanmele reviewed May 15, 2025

View reviewed changes

Licini added 3 commits May 15, 2025 10:21

clean up the function

0c866d1

factory function instead of class

7ef3e4d

docstring

55dcfae

Licini added 7 commits May 16, 2025 12:12

get_sceneobject_node

3e7a43d

more clean up and test coverage

dd4344c

lint

981464d

add coverage

85c3300

more tests

44163d3

lint

ab5425a

changelog

38a7d63

Licini requested review from gonzalocasas, chenkasirer and tomvanmele May 16, 2025 13:36

clean up kwargs

ef4def8

Licini marked this pull request as ready for review May 16, 2025 14:53

Licini commented May 16, 2025

View reviewed changes

tomvanmele reviewed May 22, 2025

View reviewed changes

naming pattern, type hints and changelog

01cf1af

Licini commented May 26, 2025

View reviewed changes

gonzalocasas added the breaking change label May 30, 2025

Licini marked this pull request as draft May 30, 2025 13:50

Scene Data Store #1461

Are you sure you want to change the base?

Scene Data Store #1461

Uh oh!

Conversation

Licini commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Scene data store

2. SceneObjectFactory

3. Others

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Licini May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomvanmele commented May 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Licini commented May 26, 2025

Uh oh!

Uh oh!

Licini commented May 14, 2025 •

edited

Loading

codecov bot commented May 15, 2025 •

edited

Loading

Licini May 16, 2025 •

edited

Loading