[RF] Remove some more classes from the public RooFit interface #12800
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guitargeek
force-pushed
the
roofit_cleanups
branch
from
3a9c0db to
a86169d
Compare
guitargeek
force-pushed
the
roofit_cleanups
branch
from
a86169d to
54d21c2
Compare
The `RooCatType` got deprcated in ROOT 6.22 with commit `560f59fbc0c`.
guitargeek
force-pushed
the
roofit_cleanups
branch
from
54d21c2 to
b2d9fe7
Compare
|
Starting build on |
lmoneta
approved these changes
May 15, 2023
Member
lmoneta
left a comment
lmoneta
left a comment
There was a problem hiding this comment.
LGTM!
Thank you Jonas for this clean-up!
Only comment is if we are sure that removing the RooFormula will not be a problem for some users
Contributor
Author
|
Weill, one can never be 100 % sure, but from searching through GitHub and CERN GitLab and googling, it looks like no user is using the |
guitargeek
added a commit
to guitargeek/root
that referenced
this pull request
May 16, 2023
With PR root-project#12800, the `RooFormula` was taken out of the public user interface, because it is an implementation detail of the RooFormulaVar and RooGenericPdf. However, there are cases where people used the RooFormula interface to access some information that was otherwise unavailable, like the set of actual variables that the formula depends on. There is some code in RooFormulaVar and RooGenericPdf that makes it seem like these classes provide an interface to the actual variables themselves: ```c++ const RooArgList& dependents() const { return _actualVars; } ... RooListProxy _actualVars ; ///< Actual parameters used by formula engine ``` However, `_actualVars` is containing **all** variables, not the actual variables as the name suggests! Fixing this removes the need to access the actual vars via the `RooFormula`. This connects to a discussion that was held on GitHub: root-project@a470a3d#commitcomment-113514331
guitargeek
added a commit
to guitargeek/root
that referenced
this pull request
May 16, 2023
With PR root-project#12800, the `RooFormula` was taken out of the public user interface, because it is an implementation detail of the RooFormulaVar and RooGenericPdf. However, there are cases where people used the RooFormula interface to access some information that was otherwise unavailable, like the set of actual variables that the formula depends on. There is some code in RooFormulaVar and RooGenericPdf that makes it seem like these classes provide an interface to the actual variables themselves: ```c++ const RooArgList& dependents() const { return _actualVars; } ... RooListProxy _actualVars ; ///< Actual parameters used by formula engine ``` However, `_actualVars` is containing **all** variables, not the actual variables as the name suggests! Fixing this removes the need to access the actual vars via the `RooFormula`. This connects to a discussion that was held on GitHub: root-project@a470a3d#commitcomment-113514331
guitargeek
added a commit
to guitargeek/root
that referenced
this pull request
May 24, 2023
With PR root-project#12800, the `RooFormula` was taken out of the public user interface, because it is an implementation detail of the RooFormulaVar and RooGenericPdf. However, there are cases where people used the RooFormula interface to access some information that was otherwise unavailable, like the set of actual variables that the formula depends on. There is some code in RooFormulaVar and RooGenericPdf that makes it seem like these classes provide an interface to the actual variables themselves: ```c++ const RooArgList& dependents() const { return _actualVars; } ... RooListProxy _actualVars ; ///< Actual parameters used by formula engine ``` However, `_actualVars` is containing **all** variables, not the actual variables as the name suggests! Fixing this removes the need to access the actual vars via the `RooFormula`. This connects to a discussion that was held on GitHub: root-project@a470a3d#commitcomment-113514331
guitargeek
added a commit
that referenced
this pull request
May 24, 2023
With PR #12800, the `RooFormula` was taken out of the public user interface, because it is an implementation detail of the RooFormulaVar and RooGenericPdf. However, there are cases where people used the RooFormula interface to access some information that was otherwise unavailable, like the set of actual variables that the formula depends on. There is some code in RooFormulaVar and RooGenericPdf that makes it seem like these classes provide an interface to the actual variables themselves: ```c++ const RooArgList& dependents() const { return _actualVars; } ... RooListProxy _actualVars ; ///< Actual parameters used by formula engine ``` However, `_actualVars` is containing **all** variables, not the actual variables as the name suggests! Fixing this removes the need to access the actual vars via the `RooFormula`. This connects to a discussion that was held on GitHub: a470a3d#commitcomment-113514331
enirolf
pushed a commit
to enirolf/root
that referenced
this pull request
May 26, 2023
With PR root-project#12800, the `RooFormula` was taken out of the public user interface, because it is an implementation detail of the RooFormulaVar and RooGenericPdf. However, there are cases where people used the RooFormula interface to access some information that was otherwise unavailable, like the set of actual variables that the formula depends on. There is some code in RooFormulaVar and RooGenericPdf that makes it seem like these classes provide an interface to the actual variables themselves: ```c++ const RooArgList& dependents() const { return _actualVars; } ... RooListProxy _actualVars ; ///< Actual parameters used by formula engine ``` However, `_actualVars` is containing **all** variables, not the actual variables as the name suggests! Fixing this removes the need to access the actual vars via the `RooFormula`. This connects to a discussion that was held on GitHub: root-project@a470a3d#commitcomment-113514331
maksgraczyk
pushed a commit
to maksgraczyk/root
that referenced
this pull request
Jun 28, 2023
With PR root-project#12800, the `RooFormula` was taken out of the public user interface, because it is an implementation detail of the RooFormulaVar and RooGenericPdf. However, there are cases where people used the RooFormula interface to access some information that was otherwise unavailable, like the set of actual variables that the formula depends on. There is some code in RooFormulaVar and RooGenericPdf that makes it seem like these classes provide an interface to the actual variables themselves: ```c++ const RooArgList& dependents() const { return _actualVars; } ... RooListProxy _actualVars ; ///< Actual parameters used by formula engine ``` However, `_actualVars` is containing **all** variables, not the actual variables as the name suggests! Fixing this removes the need to access the actual vars via the `RooFormula`. This connects to a discussion that was held on GitHub: root-project@a470a3d#commitcomment-113514331
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the continuation of the campaign to slim down the public RooFit interface:
RooCatTypeRooGenProdProjprivateRooScaledFunc.RooFormulafrom public RooFit interface