Skip to content
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

Generalize the functionality of converting parameters #102

Merged
merged 6 commits into from
Jan 17, 2025
Merged

Conversation

jmcarcell
Copy link
Member

@jmcarcell jmcarcell commented Dec 17, 2024

so that a Frame may be passed or not. This is needed to use IOSvc with the Marlin wrapper, because in that case there is not a frame that is passed around like with the PodioDataSvc.

  • Make convertObjectParameters take a function object as argument that deals with actually handling the parameter storage on the EDM4hep side. This allows to generalize its usage to cases, where a podio::Frame is not directly accessible, but some other functionality for storing parameters is available.

There are multiple ways of solving this but given that the convertObjectParameters function is only used in the wrapper I think this is the simplest one even if it changes the interface.

edit: removed the tags for the tagging script as this has been reverted again in #105 so it should not end up in the release notes.

@tmadlener
Copy link
Contributor

tmadlener commented Jan 16, 2025

I have added a second template parameter to this and introduced the ParamFramePutter helper struct that can be passed to it for putting things into a Frame. A similar struct can be used in the k4MarlinWrapper to switch to k4FWCore::putParameter instead. This makes it possible to remove the dependency on k4FWCore again.

edit: Also I have put the original overload back for convenience since it's effectively a one line wrapper.

@tmadlener tmadlener changed the title Change an argument for converting the parameters to be std::optional Generalize the functionality of converting parameters Jan 17, 2025
@jmcarcell jmcarcell merged commit 9a0ae14 into main Jan 17, 2025
7 checks passed
@jmcarcell jmcarcell deleted the add-optional branch January 17, 2025 12:32
jmcarcell added a commit that referenced this pull request Jan 30, 2025
tmadlener pushed a commit that referenced this pull request Jan 31, 2025
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