-
Notifications
You must be signed in to change notification settings - Fork 59
Make it possible to get the collection for an object #782
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
base: master
Are you sure you want to change the base?
Conversation
include/podio/Frame.h
Outdated
template <CollectionType CollT> | ||
const CollT& get(const CollT::value_type& object) const; |
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.
template <CollectionType CollT> | |
const CollT& get(const CollT::value_type& object) const; | |
template <CollectionType CollT> | |
const CollT& get(const typename CollT::value_type& object) const; |
Adding a typename here (and in definition below) shouldn't hurt (although we already value_type
to be type in the concept)
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.
Interesting that this seems to work without typename
for gcc11, but not for gcc13. Fixed.
include/podio/Frame.h
Outdated
template <ObjectType O> | ||
inline const podio::CollectionBase* get(const O& object) const; |
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.
Do we want to have this for compatibility with other get
s in frame? Otherwise, I think the objects already have typedef to their collection so in principle we could use only the overload returning concrete collection
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.
Good catch. I haven't thought about being able to return a typed pointer here. For compatibility I would keep this to return a pointer, i.e. all methods of get
that take an explicit collection template argument return a const&
, the "un-templated" ones return a const*
. Alternatively, we only add this once there is an actual use case for it.
8e2a5b4
to
c40bbaa
Compare
Link defines the value_type type member which should be object_type for conformancy with other generated handles.
c40bbaa
to
6f73308
Compare
BEGINRELEASENOTES
Frame::get
overloads that take a podio generated object handle as argument and return the collection this object belongs to if the collection is known to the Frame.ENDRELEASENOTES
See the discussion in key4hep/k4FWCore#311 for a rationale of this functionality
The main question at this point is whether the existing
getName
functionality that takes acollectionID
should be deprecated and removed eventually.