-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make sure that files with the complete datamodel can be read #361
Conversation
deae73b
to
d8efe0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this PR I don't have any more comments, I'll think about the other PR related to backwards compatibility.
# With the current version of cppyy (from ROOT 6.30.06) | ||
# it's not possible to initialize an std::array from a list | ||
# In future versions (works with 6.32.02) it will be possible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm, the cppyy version coming with ROOT 6.30.06 is 1.6.2 and with ROOT 6.32.02 is 3.1.2, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
38b9a99
to
6f68c09
Compare
- Easier for testing if the counter is reset for every type - Easier to read / maintain
Co-authored-by: Mateusz Jakub Fila <[email protected]>
4dde6b8
to
25a3bc3
Compare
It looks like |
Hmm locally I get that is loading GaudiTesting but the tests pass. The last change here doesn't change anything for me, the tests always pass. To remove |
The tests were always passing. However, the output contained things like Line 12 in 6d785eb
|
BEGINRELEASENOTES
createEDM4hepFile.py
script to facilitate testing. Apart from the obviously different values in the objects, the overall structure is almost unchanged for the output file.MCVertexRecoParticleLinkCollection
has been renamed to theVertexRecoParticleLinkCollection
subdetectorHoleNumbers
are also populated forTrack
s.ENDRELEASENOTES
I have refactored the
createEDM4hepFile.py
script to effectively reset the counter to 0 for every datatype in order to make testing a bit easier, because we won't have global changes in expected values in case some data members are added or removed. With this we can use this script to generate an EDM4hep file for every tag we have and store it somewhere (probably EOS, the same way we do it for podio) and download them on-the-fly for testing to be able to check that we don't accidentally break backwards compatibility with changes to the datamodel.This is the first part of #358 without the backwards compatibility parts yet, to disentangle the two and to make it possible to tag this part first before adding backwards compatibility tests. In this way the latter will have a clearly reproducible path to regenerating input files if necessary.