-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[RF] Remove error in RooWorkspace::getSnapshot()
if no snapshot found
#11584
Conversation
In this forum post, the point was made that `RooWorkspace::getSnapshot()` should not print an error if a snapshot with the passed name is not found. https://root-forum.cern.ch/t/roofit-check-if-snapshot-exist-without-showing-objecthandling-error Just like in the other `RooWorkspace` access functions, like `arg()`, `pdf()`, or `function()`, users expect to use `getSnapshot()` also to query if a snapshot exists and check if the returned value is `nullptr`. So there should be no error printed in `getSnapshot()` itself. If it is actually an error for th caller that no snapshot has been found, an error or exception should be raised by the caller.
Starting build on |
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
Build failed on ROOT-performance-centos8-multicore/cxx17. Failing tests: |
* replace `getSize()>0` with `!empty()` * replace manual memory management with smart pointers * replace manually allocated C-style strings with `std::string`
e53e06b
to
3e6bbb4
Compare
Starting build on |
Build failed on mac1015/cxx17. Failing tests: |
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.
LGTM!
In this forum post, the point was made that
RooWorkspace::getSnapshot()
should not print an error if a snapshotwith the passed name is not found.
https://root-forum.cern.ch/t/roofit-check-if-snapshot-exist-without-showing-objecthandling-error
Just like in the other
RooWorkspace
access functions, likearg()
,pdf()
, orfunction()
, users expect to usegetSnapshot()
also toquery if a snapshot exists and check if the returned value is
nullptr
.So there should be no error printed in
getSnapshot()
itself. If it isactually an error for th caller that no snapshot has been found, an
error or exception should be raised by the caller.
A second commit in this PR applies some general code modernization to
RooWorkspace.cxx
.