You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
There's a bunch of PODIO-related issues which may or may not have been fixed. These are smaller complaints that pop up on comment threads and might otherwise fall through the cracks. None of them presumably affect EICrecon, but we still want our implementation to be solid.
Set<>() doesn't check if the collection is already in the frame, and relies on PODIO to throw an exception with a sensible error message. This risks leaving the JFactory in a messed-up state. Usually this isn't a problem because exceptions kill the whole JANA run(), but it might become a problem if the user catches and swallows exceptions inside their JFactory. (Which, to be clear, they really shouldn't, but we have to assume someone out there will do that and have a good(ish) reason to.
Insert() forgets to set mData. Ideally we'd get rid of Insert() entirely, but that's probably too much of a breaking change.
Also, as explained in a comment in EICrecon/nbrei_omnifactories: JOmniFactoryGeneratorT.h: 103:
The tag set via JMultiFactory::SetTag() is automatically applied as a suffix to all of the collection names? This is definitely not the behavior we want for JOmniFactories and might not be the behavior we want anywhere. Need to research what is happening with the other JMultiFactories currently in use in EICrecon
Did JMultiFactory ever get a usable m_app and m_plugin_name? If so, we can remove the hacked version in JChainMultiFactoryT. Also, JMultifactory doesn't appear to catch and re-throw std::exceptions, which it is supposed to. Finally, multifactories appear to not be providing empty collections to podio in the case of the user not populating anything. (See comment in JOmniFactoryTests.cc.)
There's a bunch of PODIO-related issues which may or may not have been fixed. These are smaller complaints that pop up on comment threads and might otherwise fall through the cracks. None of them presumably affect EICrecon, but we still want our implementation to be solid.
Set<>() doesn't check if the collection is already in the frame, and relies on PODIO to throw an exception with a sensible error message. This risks leaving the JFactory in a messed-up state. Usually this isn't a problem because exceptions kill the whole JANA run(), but it might become a problem if the user catches and swallows exceptions inside their JFactory. (Which, to be clear, they really shouldn't, but we have to assume someone out there will do that and have a good(ish) reason to.
Insert() forgets to set mData. Ideally we'd get rid of Insert() entirely, but that's probably too much of a breaking change.
#243 (comment)
The text was updated successfully, but these errors were encountered: