Add python bindings for the datamodel classes

[jmcarcell]

BEGINRELEASENOTES

• Add python bindings for the datamodel classes and some documentation on how to use the bindings

ENDRELEASENOTES

See the README for examples on how to use them

[tmadlener]

Thanks for taking care of this. This already looks pretty good, My main complaint is the factory being something that the users can see. I suppose we need the factory because of the dynamic nature of ROOTs python bindings, right? Can we still try to "hide" it in the __init__.py or in datamodel.py such that it is already instantiated and what the user sees would look something like:

from edm4hep import edm4hep
particle = edm4hep.MCParticle()
# ...

This should also work with from edm4hep import *, but I am not sure I like that better. Ideally it would just be import edm4hep and then we have the factory in hand, but I don't see a way how that could work.

[jmcarcell]

It has been changed now to edm4hep so both work:

>>> from edm4hep import *
>>> edm4hep
<edm4hep.datamodel.Datamodel object at 0x7f4bd08fa0b0>
>>> edm4hep.MCParticle()
<cppyy.gbl.edm4hep.MCParticle object at 0x55b3f52c4710>

or the one in the README:

>>> from edm4hep import edm4hep
>>> edm4hep.MCParticle()
<cppyy.gbl.edm4hep.MCParticle object at 0x55b4034ba030>

import edm4hep doesn't work since it's the top level module... And maybe it's not a good idea in case we have other python tools in the future for edm4hep

[tmadlener]

Thanks, this looks very good now. I just realized we do have some tests for utility functionality already, that redo some of the stuff that is introduced here:

EDM4hep/test/utils/test_kinematics.py

Lines 10 to 16 in 1145e20

	if ROOT.gSystem.Load('libedm4hepDict.so') < 0:
	print('Cannot load edm4hep dictionary for tests')
	sys.exit(1)
	ROOT.gInterpreter.LoadFile(os.path.dirname(__file__) + '/../../utils/include/edm4hep/utils/kinematics.h')
	from ROOT import edm4hep

Now that we have a proper place for an __init__.py maybe we could even use that to also include the JIT loading of the utils/kinematics.h header properly instead of the hackish way we do it at the moment for the tests.

[tmadlener]

Even more cases in the tests that could be cleaned up:

EDM4hep/test/test_rdf.py

Lines 5 to 9 in 1145e20

	# Load the implementations of the utility functions
	ROOT.gSystem.Load('libedm4hepRDF.so')
	# Load the declarations of the utility functions to make them available for JIT
	# compilation
	ROOT.gInterpreter.LoadFile('edm4hep/utils/dataframe.h')

[jmcarcell]

That is done now. I think it could be done on demand when using python but in the cases where something in edm4hep::utils is passed as a string python has no way to know so this will always load both headers. For running the tests manually it's a bit annoying that one has to set the paths but it's hard to make it possible to load the headers always. I tried and had a working version for tests and runtime without needing to set the paths but then someone could break it by installing instead of in install/ in the main tree in install1/install2 or making an in-source build

[tmadlener]

Can we make this work like this?

from edm4hep import utils
p4 = utils.p4

or

import edm4hep
p4 = edm4hep.utils.p4

Or is this already possible?

[jmcarcell]

I made some changes in ad539e7 and now it's possible:

from edm4hep import edm4hep
particle = edm4hep.MCParticle() # default initialized particle
particle.getCharge() # 0.0

from edm4hep import utils
p  = utils.p4(particle) # edm4hep.utils.p4 or utils.p4 works
print(p) # (0,0,0,0)

There are two classes now, one for serving all classes from edm4hep and another one for the utils namespace. If there were more namespaces it's easy to change to a dictionary-based class that would store each of the edm4hep.namespace object and serve by doing a lookup on the dictionary

[tmadlener]

Very nice, thanks a lot for having another look at this. Since we are already at it, I think we could also add a __version__ to the __init__.py for an even more pythonic feel.

In podio we have a __init__.py.in that we configure during the cmake stage. This should be trivial to add here as well, I think.

• configure_file in cmake
• __init__.py.in

[tmadlener]

I am wondering whether all these RuntimeErrors should be ImportErrors instead. In the end import edm4hep is the thing that will fail if one of these is triggered.

[jmcarcell]

Well typically for the libraries it will be because they are not in LD_LIBRARY_PATH which is a runtime search so one could say it's a RuntimeError ... both are quite generic, I don't think ImportError tells you much, that will also be quite obveious when checking the trace. So I don't have a strong opinion on this.

[tmadlener]

Yeah, maybe a RuntimeError is better, because that doesn't point you towards a python problem in this case.

[jmcarcell]

I added a version file. I preferred that over having an __init__.py.in so that it's possible to have the module loaded when developing and have suggestions, fixes, etc right away:

and if the version is not there it will complain but keep loading everything else.

[tmadlener]

Really final comment (:wink:): I think it would make sense to add python/edm4hep/__version__.py to .gitignore

request has been addressed