Skip to content

Conversation

tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented May 20, 2025

BEGINRELEASENOTES

  • Add 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 a collectionID should be deprecated and removed eventually.

Comment on lines 210 to 211
template <CollectionType CollT>
const CollT& get(const CollT::value_type& object) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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)

Copy link
Collaborator Author

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.

Comment on lines 222 to 223
template <ObjectType O>
inline const podio::CollectionBase* get(const O& object) const;
Copy link
Contributor

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 gets 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

Copy link
Collaborator Author

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.

@tmadlener tmadlener force-pushed the frame-get-from-object branch 3 times, most recently from 8e2a5b4 to c40bbaa Compare May 20, 2025 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants