From d12cf45698f9764cee061b19acc73afd5bf22b67 Mon Sep 17 00:00:00 2001
From: Mateusz Jakub Fila <37295697+m-fila@users.noreply.github.com>
Date: Sat, 24 Feb 2024 12:59:38 +0100
Subject: [PATCH] Fix: mutable to immutable object conversion with python
 bindings (#564)

* added python bindings test for mutable object conversion

* replaced object conversion function with converting constructor

* ignore emplace_back clang-tidy suggestion in test
---
 python/podio/test_CodeGen.py             | 33 ++++++++++++++++++++++++
 python/templates/MutableObject.cc.jinja2 |  1 -
 python/templates/MutableObject.h.jinja2  |  3 ---
 python/templates/Object.cc.jinja2        |  2 ++
 python/templates/Object.h.jinja2         |  2 ++
 tests/unittests/unittest.cpp             |  2 +-
 6 files changed, 38 insertions(+), 5 deletions(-)
 create mode 100644 python/podio/test_CodeGen.py

diff --git a/python/podio/test_CodeGen.py b/python/podio/test_CodeGen.py
new file mode 100644
index 000000000..3f1328c22
--- /dev/null
+++ b/python/podio/test_CodeGen.py
@@ -0,0 +1,33 @@
+#!/usr/bin/env python3
+"""cppyy python binding tests"""
+
+import unittest
+import ROOT
+from ROOT import ExampleMCCollection, MutableExampleMC
+
+
+class ObjectConversionsTest(unittest.TestCase):
+    """Object conversion binding tests"""
+
+    def test_conversion_mutable_to_immutable(self):
+        ROOT.gInterpreter.Declare(
+            """
+            void test_accepts_immutable(ExampleMC) {}
+            """
+        )
+        accepts_immutable = ROOT.test_accepts_immutable
+        mutable_obj = MutableExampleMC()
+        accepts_immutable(mutable_obj)
+
+
+class OneToManyRelationsTest(unittest.TestCase):
+    """OneToManyRelations binding tests"""
+
+    def test_add(self):
+        particles = ExampleMCCollection()
+        parent_particle = particles.create()
+        daughter_particle = particles.create()
+
+        self.assertEqual(len(daughter_particle.parents()), 0)
+        daughter_particle.addparents(parent_particle)
+        self.assertEqual(len(daughter_particle.parents()), 1)
diff --git a/python/templates/MutableObject.cc.jinja2 b/python/templates/MutableObject.cc.jinja2
index bb851a48c..eaec2f716 100644
--- a/python/templates/MutableObject.cc.jinja2
+++ b/python/templates/MutableObject.cc.jinja2
@@ -17,7 +17,6 @@
 {{ utils.namespace_open(class.namespace) }}
 
 {{ macros.constructors_destructors(class.bare_type, Members, prefix='Mutable') }}
-Mutable{{ class.bare_type }}::operator {{ class.bare_type }}() const { return {{ class.bare_type }}(m_obj); }
 
 {{ macros.member_getters(class, Members, use_get_syntax, prefix='Mutable') }}
 {{ macros.single_relation_getters(class, OneToOneRelations, use_get_syntax, prefix='Mutable') }}
diff --git a/python/templates/MutableObject.h.jinja2 b/python/templates/MutableObject.h.jinja2
index 086f99b8b..19af5e3a3 100644
--- a/python/templates/MutableObject.h.jinja2
+++ b/python/templates/MutableObject.h.jinja2
@@ -39,9 +39,6 @@ public:
 
 {{ macros.constructors_destructors(class.bare_type, Members, prefix='Mutable') }}
 
-  /// conversion to const object
-  operator {{ class.bare_type }}() const;
-
 public:
 
 {{ macros.member_getters(Members, use_get_syntax) }}
diff --git a/python/templates/Object.cc.jinja2 b/python/templates/Object.cc.jinja2
index a526162ac..e95548fcc 100644
--- a/python/templates/Object.cc.jinja2
+++ b/python/templates/Object.cc.jinja2
@@ -18,6 +18,8 @@
 
 {{ macros.constructors_destructors(class.bare_type, Members) }}
 
+{{ class.bare_type }}::{{ class.bare_type }}(const Mutable{{ class.bare_type }}& other): {{ class.bare_type }}(other.m_obj) {}
+
 {{ class.bare_type }}::{{ class.bare_type }}({{ class.bare_type }}Obj* obj) : m_obj(podio::utils::MaybeSharedPtr<{{ class.bare_type }}Obj>(obj)) {}
 
 {{ class.bare_type }} {{ class.bare_type }}::makeEmpty() {
diff --git a/python/templates/Object.h.jinja2 b/python/templates/Object.h.jinja2
index 17afef9cd..acee909be 100644
--- a/python/templates/Object.h.jinja2
+++ b/python/templates/Object.h.jinja2
@@ -43,6 +43,8 @@ public:
   using collection_type = {{ class.bare_type }}Collection;
 
 {{ macros.constructors_destructors(class.bare_type, Members) }}
+  /// converting constructor from mutable object
+  {{ class.bare_type }}(const Mutable{{ class.bare_type }}& other);
 
   static {{ class.bare_type }} makeEmpty();
 
diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp
index d26523b5d..74cbded14 100644
--- a/tests/unittests/unittest.cpp
+++ b/tests/unittests/unittest.cpp
@@ -156,7 +156,7 @@ TEST_CASE("Container lifetime", "[basics][memory-management]") {
   {
     MutableExampleHit hit;
     hit.energy(3.14f);
-    hits.push_back(hit);
+    hits.push_back(hit); // NOLINT(modernize-use-emplace)
   }
   auto hit = hits[0];
   REQUIRE(hit.energy() == 3.14f);