Repository: pycollada/pycollada

Issue:
Allow to set the <node> name and id attributes
The collada specs (V1.4) defines, that the `<node>` element might have two attributes:
- `id` (optional, but if present must be unique)
- `name` (optional, to name the node, duplicate names allowed)

Unfortunately the `collada.scene.Node` class in pycollada only has the `id` property - when writing out the object as XML, it uses the `id` value to write the `id` and `name`  XML attributes:

https://github.com/pycollada/pycollada/blob/c1987ed630b8161049c034406a0b1fc99e1ccd7e/collada/scene.py#L391-L393

Would it be possible to add an additional `name` property to the `collada.scene.Node` class? I guess this could be empty by default (for backwards compatibility) and only be used if set externally.

The use case for this is to keep model node names, especially when converting from another format (I use FreeCAD to convert STEP models into DAE and now loose the STEP model component tree names when converting - FreeCAD has all names in its internal data structure, but has no way to set them when exporting to DAE (it is using pycollada)). 

Thanks!

Hints from issue comments:
@jterrace: Yes, I don't think it would be too difficult to add. Pull requests welcome.

Solution patch:
diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml
index 5bb6f27..829d572 100644
--- a/.github/workflows/python-package.yml
+++ b/.github/workflows/python-package.yml
@@ -10,6 +10,7 @@ on:
   pull_request:
     branches:
      - master
+  workflow_dispatch:
 
 jobs:
   build:
@@ -40,4 +41,4 @@ jobs:
         flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
     - name: Test with pytest
       run: |
-        pytest
\ No newline at end of file
+        pytest
diff --git a/collada/scene.py b/collada/scene.py
index 3cba88b..849710f 100644
--- a/collada/scene.py
+++ b/collada/scene.py
@@ -314,7 +314,7 @@ class Node(SceneNode):
     Contains the list of transformations effecting the node as well as any children.
     """
 
-    def __init__(self, id, children=None, transforms=None, xmlnode=None):
+    def __init__(self, id, children=None, transforms=None, xmlnode=None, name=None):
         """Create a node in the scene graph.
 
         :param str id:
@@ -327,10 +327,15 @@ class Node(SceneNode):
           contain any object that inherits from :class:`collada.scene.Transform`
         :param xmlnode:
           When loaded, the xmlnode it comes from
+        :param name:
+          If given, sets the node name property, if missing (None) then then
+          node name property is set from the id value (if not None)
 
         """
         self.id = id
         """The unique string identifier for the node"""
+        self.name = id if name is None else name
+        """The optional string node name (not required to be unique)for the node, by default set to the id value"""
         self.children = []
         """A list of child nodes of this node. This can contain any
           object that inherits from :class:`collada.scene.SceneNode`"""
@@ -353,7 +358,7 @@ class Node(SceneNode):
             self.xmlnode = xmlnode
             """ElementTree representation of the transform."""
         else:
-            self.xmlnode = E.node(id=self.id, name=self.id)
+            self.xmlnode = E.node(id=self.id, name=self.name)
             for t in self.transforms:
                 self.xmlnode.append(t.xmlnode)
             for c in self.children:
@@ -390,7 +395,9 @@ class Node(SceneNode):
 
         if self.id is not None:
             self.xmlnode.set('id', self.id)
-            self.xmlnode.set('name', self.id)
+        if self.name is not None:
+            self.xmlnode.set('name', self.name)
+            
         for t in self.transforms:
             if t.xmlnode not in self.xmlnode:
                 self.xmlnode.append(t.xmlnode)
@@ -406,6 +413,7 @@ class Node(SceneNode):
     @staticmethod
     def load( collada, node, localscope ):
         id = node.get('id')
+        name = node.get('name')
         children = []
         transforms = []
 
@@ -419,7 +427,7 @@ class Node(SceneNode):
             except DaeError as ex:
                 collada.handleError(ex)
 
-        return Node(id, children, transforms, xmlnode=node)
+        return Node(id, children, transforms, xmlnode=node, name=name)
 
     def __str__(self):
         return '<Node transforms=%d, children=%d>' % (len(self.transforms), len(self.children))


Test patch:
diff --git a/collada/tests/test_scene.py b/collada/tests/test_scene.py
index 076a154..e153fe4 100644
--- a/collada/tests/test_scene.py
+++ b/collada/tests/test_scene.py
@@ -169,6 +169,7 @@ class TestScene(unittest.TestCase):
         scale = collada.scene.ScaleTransform(0.1, 0.2, 0.3)
         mynode = collada.scene.Node('mynode', children=[myemptynode], transforms=[rotate, scale])
         self.assertEqual(mynode.id, 'mynode')
+        self.assertEqual(mynode.name, 'mynode')
         self.assertEqual(mynode.children[0], myemptynode)
         self.assertEqual(mynode.transforms[0], rotate)
         self.assertEqual(mynode.transforms[1], scale)
@@ -183,6 +184,7 @@ class TestScene(unittest.TestCase):
 
         yournode = collada.scene.Node.load(self.dummy, fromstring(tostring(mynode.xmlnode)), {})
         self.assertEqual(yournode.id, 'yournode')
+        self.assertEqual(yournode.name, 'mynode')
         self.assertEqual(len(yournode.children), 2)
         self.assertEqual(len(yournode.transforms), 2)
         self.assertEqual(yournode.children[0].id, 'myemptynode')
@@ -190,6 +192,30 @@ class TestScene(unittest.TestCase):
         self.assertTrue(type(yournode.transforms[0]) is collada.scene.ScaleTransform)
         self.assertTrue(type(yournode.transforms[1]) is collada.scene.TranslateTransform)
 
+        translate = collada.scene.TranslateTransform(0.1, 0.2, 0.3)
+        mynode.transforms.append(translate)
+        mynode.transforms.pop(0)
+        youremptynode = collada.scene.Node('youremptynode')
+        mynode.children.append(youremptynode)
+        mynode.id = 'yournode'
+        mynode.name = 'yourname'
+        mynode.save()
+
+        yournode = collada.scene.Node.load(self.dummy, fromstring(tostring(mynode.xmlnode)), {})
+        self.assertEqual(yournode.id, 'yournode')
+        self.assertEqual(yournode.name, 'yourname')
+        
+        myemptynode = collada.scene.Node('myemptynode')
+        rotate = collada.scene.RotateTransform(0.1, 0.2, 0.3, 90)
+        scale = collada.scene.ScaleTransform(0.1, 0.2, 0.3)
+        mynode = collada.scene.Node('mynode', children=[myemptynode], transforms=[rotate, scale])
+        self.assertEqual(mynode.id, 'mynode')
+        self.assertEqual(mynode.name, 'mynode')
+        mynode = collada.scene.Node('othernode', children=[myemptynode], transforms=[rotate, scale], name='othername')
+        self.assertEqual(mynode.id, 'othernode')
+        self.assertEqual(mynode.name, 'othername')
+        
+
     def test_scene_material_node(self):
         binding1 = ("TEX0", "TEXCOORD", "0")
         binding2 = ("TEX1", "TEXCOORD", "1")