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

No SetSingle for JMultiFactory #226

Closed
DraTeots opened this issue Jun 18, 2023 · 5 comments
Closed

No SetSingle for JMultiFactory #226

DraTeots opened this issue Jun 18, 2023 · 5 comments

Comments

@DraTeots
Copy link
Collaborator

At least in non PODIO version

@nathanwbrei
Copy link
Collaborator

Technically there is no SetSingle in JFactoryT, either. JFactoryT::Insert(T*) serves the purpose of SetSingle, however the name Insert implies you can call it multiple times to insert data one item at a time, which PODIO resoundingly rejects. I'd rather not be constantly explaining to users why JMultifactory::Insert works for non-PODIO data and throws an exception for PODIO datatypes. We could add a SetDatum(std::string tag, T* datum) but I'm worried this gives users too many things that are similar but slightly different to have to think about. What argument in favor of adding this did you have in mind?

@nathanwbrei
Copy link
Collaborator

I guess we could technically accumulate everything passed to Insert() into a std::vector, and not add it to a PODIO collection until JMultifactory::Process finishes. This will be annoying to implement and test though. Is it worth it?

@faustus123
Copy link
Collaborator

Could we just throw the exception for PODIO types, but give it a long-ish message explaining that it is a limitation of PODIO and therefore not allowed for PODIO data types? This should save you from having to explain it. I'm only suggesting this if it is a quick/easy thing to implement.

@DraTeots
Copy link
Collaborator Author

DraTeots commented Jul 22, 2023

What I don't like about it is that there is like 2 JANA2-s, one with PODIO and one without. They behave differently and their user expectations are different. It basically forces you to write different code for both (I am not touching event model here). Initially it looked like implementation compromise but as it goes further it starts to look like major architectural flow.

@nathanwbrei
Copy link
Collaborator

Moving this discussion to #254

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

No branches or pull requests

3 participants