Make sure builtin member types can also be renamed for ROOT (#799)

* Add a failing test for a datatype member rename

Fix generation script enough to make things compile again

* Add clarifying comments

* Make sure also handle renamed members that are not components

Author: tmadlener

python/podio_gen/cpp_generator.py

@@ -388,23 +388,42 @@ def _preprocess_schema_evolution_datatype(self, name, datatype):
         """
         schema_evolution_datatype = deepcopy(datatype)
         needs_schema_evolution = False
+        # We have to figure out whether we need to do schema evolution and how
+        # we do it. We have to do it if either a (non-component) member is
+        # affected or if a member that is a component type has a renamed member.
+        # The things we need to do differ slightly in both cases
         for member in schema_evolution_datatype["Members"]:
             member_type = member.array_type if member.is_array else member.full_type
-            if member_type in self.root_schema_dict:
-                needs_schema_evolution = True
-                replace_component_in_paths(
-                    member_type,
-                    _versioned(member_type, self.old_schema_version),
-                    schema_evolution_datatype["includes_data"],
-                )
-                if member.is_array:
-                    member.full_type = member.full_type.replace(
-                        member.array_type, _versioned(member.array_type, self.old_schema_version)
+            for type_name, evolutions in self.root_schema_dict.items():
+                if type_name == member_type:
+                    needs_schema_evolution = True
+                    # We have a component with a member that got renamed. Hence we
+                    # need to make sure we generate a version of the Data class with
+                    # the old component
+                    replace_component_in_paths(
+                        member_type,
+                        _versioned(member_type, self.old_schema_version),
+                        schema_evolution_datatype["includes_data"],
                     )
-                    member.array_type = _versioned(member.array_type, self.old_schema_version)
-                else:
-                    member.full_type = _versioned(member.full_type, self.old_schema_version)
-                    member.bare_type = _versioned(member.bare_type, self.old_schema_version)
+                    if member.is_array:
+                        member.full_type = member.full_type.replace(
+                            member.array_type,
+                            _versioned(member.array_type, self.old_schema_version),
+                        )
+                        member.array_type = _versioned(member.array_type, self.old_schema_version)
+                    else:
+                        member.full_type = _versioned(member.full_type, self.old_schema_version)
+                        member.bare_type = _versioned(member.bare_type, self.old_schema_version)
+                for evolution in evolutions:
+                    if (
+                        isinstance(evolution, RenamedMember)
+                        and member.name == evolution.member_name_new
+                    ):
+                        # We have a member that has been renamed. We just need to
+                        # make sure we get a version of the Data class with the old
+                        # name
+                        needs_schema_evolution = True
+                        member.name = evolution.member_name_old
 
         if needs_schema_evolution:
             print(f"  Preparing explicit schema evolution for {name}")
@@ -459,7 +478,11 @@ def _prepare_iorules(self):
             for schema_change in schema_changes:
                 if isinstance(schema_change, RenamedMember):
                     # find out the type of the renamed member
-                    component = self.datamodel.components[type_name]
+                    component = self.datamodel.components.get(type_name)
+                    is_datatype = False
+                    if component is None:
+                        is_datatype = True
+                        component = self.datamodel.datatypes[type_name]
                     member_type = None
                     for member in component["Members"]:
                         if member.name == schema_change.member_name_new:
@@ -473,6 +496,10 @@ def _prepare_iorules(self):
                     iorule = RootIoRule()
                     iorule.sourceClass = type_name
                     iorule.targetClass = type_name
+                    if is_datatype:
+                        iorule.sourceClass = f"{type_name}Data"
+                        iorule.targetClass = f"{type_name}Data"
+
                     iorule.version = self.old_schema_version
                     iorule.source = f"{member_type} {schema_change.member_name_old}"
                     iorule.target = schema_change.member_name_new
@@ -601,10 +628,8 @@ def _create_selection_xml(self):
             "version": self.datamodel.schema_version,
             "components": [DataType(c) for c in self.datamodel.components],
             "datatypes": [DataType(d) for d in self.datamodel.datatypes],
-            "old_schema_components": [
-                DataType(d)
-                for d in self.root_schema_datatype_names | self.root_schema_component_names
-            ],
+            "old_schema_components": [DataType(d) for d in self.root_schema_component_names],
+            "old_schema_datatypes": [DataType(d) for d in self.root_schema_datatype_names],
             "iorules": self.root_schema_iorules,
         }
 

python/templates/selection.xml.jinja2

@@ -38,6 +38,9 @@
         <!-- previous schema components and pods -->
 {% for class in old_schema_components %}
 {{ class_selection(class) }}
+{% endfor %}
+{% for class in old_schema_datatypes %}
+{{ class_selection(class, postfix="Data") }}
 {% endfor %}
 
     </selection>

tests/schema_evolution/datalayout_new.yaml

@@ -91,7 +91,7 @@ datatypes :
     Description : "Cluster"
     Author : "B. Hegner"
     Members:
-      - double energy // cluster energy
+      - double newEnergyName // cluster energy (renamed for testing purposes)
     OneToManyRelations:
      - ExampleHit Hits // hits contained in the cluster
      - ExampleCluster Clusters // sub clusters used to create this cluster

tests/schema_evolution/read_new_data.h

@@ -1,6 +1,7 @@
 #ifndef PODIO_TESTS_SCHEMAEVOLUTION_READNEWDATA_H // NOLINT(llvm-header-guard): folder structure not suitable
 #define PODIO_TESTS_SCHEMAEVOLUTION_READNEWDATA_H // NOLINT(llvm-header-guard): folder structure not suitable
 
+#include "datamodel/ExampleClusterCollection.h"
 #include "datamodel/ExampleHitCollection.h"
 #include "datamodel/ExampleWithARelationCollection.h"
 #include "datamodel/ExampleWithNamespaceCollection.h"
@@ -30,6 +31,14 @@ int readExampleHit(const podio::Frame& event) {
   return 0;
 }
 
+int readExampleCluster(const podio::Frame& event) {
+  const auto& coll = event.get<ExampleClusterCollection>("datatypeMemberRenameTest");
+  const auto elem = coll[0];
+  ASSERT_EQUAL(elem.newEnergyName(), 3.14, "Renamed datatype members does not have the expected value");
+
+  return 0;
+}
+
 int readExampleWithNamespace(const podio::Frame& event) {
   const auto& coll = event.get<ex42::ExampleWithNamespaceCollection>("componentMemberRenameTest");
   auto elem = coll[0];
@@ -59,6 +68,7 @@ int read_new_data(const std::string& filename) {
 
   int result = 0;
   result += readExampleHit(event);
+  result += readExampleCluster(event);
   result += readExampleWithNamespace(event);
   result += readExampleWithARelation(event);
 

tests/schema_evolution/schema_evolution.yaml

@@ -11,3 +11,8 @@ evolutions:
 
   EventInfo:
     class_renamed_to: EventInfoNewName
+
+  ExampleCluster:
+    member_rename:
+      - energy
+      - newEnergyName

tests/schema_evolution/write_old_data.h

@@ -1,6 +1,7 @@
 #ifndef PODIO_TESTS_SCHEMAEVOLUTION_WRITEOLDDATA_H // NOLINT(llvm-header-guard): folder structure not suitable
 #define PODIO_TESTS_SCHEMAEVOLUTION_WRITEOLDDATA_H // NOLINT(llvm-header-guard): folder structure not suitable
 
+#include "datamodel/ExampleClusterCollection.h"
 #include "datamodel/ExampleHitCollection.h"
 #include "datamodel/ExampleWithARelationCollection.h"
 #include "datamodel/ExampleWithNamespaceCollection.h"
@@ -20,6 +21,12 @@ auto writeExampleHit() {
   return coll;
 }
 
+auto writeExampleCluster() {
+  ExampleClusterCollection coll;
+  auto elem = coll.create(3.14);
+  return coll;
+}
+
 auto writeExampleWithNamespace() {
   ex42::ExampleWithNamespaceCollection coll;
   auto elem = coll.create();
@@ -40,6 +47,7 @@ auto writeExampleWithARelation() {
 podio::Frame createFrame() {
   podio::Frame event;
 
+  event.put(writeExampleCluster(), "datatypeMemberRenameTest");
   event.put(writeExampleHit(), "datatypeMemberAdditionTest");
   event.put(writeExampleWithNamespace(), "componentMemberRenameTest");
   event.put(writeExampleWithARelation(), "floatToDoubleMemberTest");