-
Notifications
You must be signed in to change notification settings - Fork 35
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
Adding track nholes to EDM #147
Conversation
Mmh, this was meant to go to the MuonColliderSoft repository (sorry I misclicked in the PR creation). Cheers, |
Changed the __iter__ method to follow the Python 3 convention
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.
From a quick glance, the main missing things are missing version checks in the readers to not break the reading of older files.
Co-authored-by: Thomas Madlener <[email protected]>
Co-authored-by: Thomas Madlener <[email protected]>
@tmadlener I have added the version checks. What else do we need to move forward? |
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.
This looks to be in quite good shape now. I have only one real comment about the version check (see below). @gaede the I/O parts of the implementation look good to me, but maybe I missed something?
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.
In order to make CI pass again, we have to bump the version manually here:
Lines 14 to 16 in e8c5032
SET( LCIO_VERSION_MAJOR 2 ) | |
SET( LCIO_VERSION_MINOR 21 ) | |
SET( LCIO_VERSION_PATCH 0 ) |
I would go to 2.22.99
to sort of indicate that this is a "development" version.
done |
BEGINRELEASENOTES
Nholes
and andsubdetectorHoleNumbers
to theTrack
for keeping track of missing hits in a Track.ENDRELEASENOTES
Inspired by today's discussion at the Joint Tracker and Calorimeter Meeting https://agenda.infn.it/event/31242/,
I have added the number of holes to the LCIO track EDM