-
Notifications
You must be signed in to change notification settings - Fork 60
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
Document I/O interfaces #648
Conversation
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.
Thanks a lot!
Just a few minor remarks. Except for the exception handling they could be covered in the same PR
include/podio/Reader.h
Outdated
/// data from the passed files | ||
/// | ||
/// @throws std::runtime_error in case the file extensions differ or in case | ||
/// support for the necessary I/O backend has not been built | ||
Reader makeReader(const std::vector<std::string>& filename); |
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.
actually it should be filenames
, not filename
include/podio/Reader.h
Outdated
/// reading. All files are assumed to be of the same I/O format, no switching | ||
/// between formats is possible. | ||
/// | ||
/// @note For SIO files this will only read the first file! |
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.
we should put in some exception handling or something of that kind. That is too burried in the docs.
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.
You mean, throw an exception when more than one SIO file is passed?
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.
we should not silently ignore faulty states.
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.
so yes
/// backend specific readers that this class wraps. In contrast to the lower | ||
/// level readers that usually return arbitrary FrameData, this interface class | ||
/// will return fully constructed Frames. | ||
/// |
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.
In addition it provides convenience methods to deal specifically with the event frame category...
Thanks for checking. I think I addressed all of them. |
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.
That was quick! thanks
BEGINRELEASENOTES
Reader
andWriter
interface classesmakeReader
throws an exception if multiple SIO files are passed (see #647)ENDRELEASENOTES