Fix: mutable to immutable object conversion with python bindings

[m-fila]

BEGINRELEASENOTES

• Fix mutable to immutable object conversion with python bindings

ENDRELEASENOTES

As reported in key4hep/EDM4hep#270 the python bindings were unable to implicitly convert mutable to immutable objects with the converting constructor. An alternative approach with converting constructor instead of conversion function is viable.

Converting constructor was added and conversion function removed to avoid ambiguity in precedence.

I'm not sure if the test is placed in a correct place

Closes key4hep/EDM4hep#270

[tmadlener]

It looks like having a converting constructor instead of a conversion operator makes clang-tidy a bit more sensitive: https://github.com/AIDASoft/podio/actions/runs/8016200074/job/21897701922?pr=564#step:4:450

[m-fila]

wlav/cppyy#218 explains inefficiencies of implicit casting in cppyy.
I tried a naive benchmark:

from timeit import timeit
from ROOT import MutableExampleMC
from cppyy.ll import static_cast
 
def bench_cast():
    parent = MutableExampleMC()
    daughter = MutableExampleMC()
    daughter.addparents(static_cast['ExampleMC'](parent))
 
def bench_no_cast():
    parent = MutableExampleMC()
    daughter = MutableExampleMC()
    daughter.addparents(parent)

def report(result, reps, name):
    print(f'Average time: {result/reps:.1E} seconds\t({name})')

if __name__ == '__main__':
    reps= 100000
    result = timeit('bench_cast()', setup='from __main__ import bench_cast; bench_cast()', number=reps)
    report(result, reps, 'explicit cast')
    result = timeit('bench_no_cast()', setup='from __main__ import bench_no_cast; bench_no_cast()', number=reps)
    report(result, reps, 'implicit cast')

With the results:

Average time: 2.7E-06 seconds   (explicit cast)
Average time: 2.2E-06 seconds   (implicit cast)

Not sure why implicit seems to be faster (very little overloads to consider?).
Regardless, I'd say there is no visible drop in performance for this case

[tmadlener]

Thanks for checking some potential performance implications. That looks reasonable to me. In the end, I don't think performance is the main concern when you go to the python bindings ;)

[jmcarcell]

This works fine in EDM4hep without the casts, I tested the script that is in key4hep/EDM4hep#272. Re the benchmarks wlav mentions all the process where you have to first fail (in a try - except block probably) and then find the correct operator and then use it. So it's going to be worse than your .5 / 2.2 = 23% loss

[tmadlener]

@wdconinc this should be a completely transparent change as far as we are aware. Would you like to give it a go before we merge, or should we just go ahead and you will report back in case you find an issue on your side?

[wdconinc]

this should be a completely transparent change as far as we are aware

Hyrum's law strikes again. Go ahead and merge. We'll consider it a bug on our end :-)