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

[RF] Deprecate some RooDataSet constructors #14317

Merged
merged 5 commits into from
Jan 15, 2024

Conversation

guitargeek
Copy link
Contributor

The other constructors can cause some confusion to the users.

For now, this change is only to see the result of the CI.

@phsft-bot
Copy link
Collaborator

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu2204/nortcxxmod.
Running on root-ubuntu-2204-3.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-performance-centos8-multicore/soversion.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Errors:

  • [2024-01-09T14:03:25.861Z] 1272/2456 Test [cxxmodules] CIFactory now uses MultiplexConsumer #988: tutorial-roostats-rs701_BayesianCalculator ........................................................***Failed Error regular expression found in output. Regex=[: error:] 0.60 sec

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on windows10/default.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-3.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

Avoid using some redundant RooDataSet constructors that will be
deprecated.
Avoid using some redundant RooDataSet constructors that will be
deprecated.

Separate commit from the previous one so it can be backported.
When using the `RooFit::Cut("...")` command argument in the RooDataSet
constructor to filter imported datasets, the `RooFormulaVar`
representing the cut should not check if all the variables in the
dataset are used for the expression.

This is already done in other code branches like the
`RooVectorDataStore` constructors, and this commit adds this tweak also
to the remaining `RooDataSet` constructors to make the behavior
consistent.
The RooDataSet has some constructors that are redundant with specific
cases that can be covered with the command-argument constructor.

This can cause confusion like here in the forum:
https://root-forum.cern.ch/t/how-to-use-weighted-unbinned-dataset-in-roofit/57631

To avoid this interface duplication, these constructors are deprecated
and will be removed in ROOT 6.34.
The correct signature is `RooFit::Import(TTree&)`.

This was noticed thanks to the following forum post.
https://root-forum.cern.ch/t/how-to-use-weighted-unbinned-dataset-in-roofit/57631
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

@guitargeek guitargeek marked this pull request as ready for review January 9, 2024 15:02
Copy link

github-actions bot commented Jan 9, 2024

Test Results

     9 files       9 suites   1d 22h 14m 9s ⏱️
 2 487 tests  2 487 ✅ 0 💤 0 ❌
21 351 runs  21 351 ✅ 0 💤 0 ❌

Results for commit d3086d4.

@guitargeek guitargeek changed the title [RF] Always use RooDataSet constructor with command arguments [RF] Deprecate some RooDataSet constructors Jan 9, 2024
Copy link
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@guitargeek guitargeek merged commit 2e7aa7c into root-project:master Jan 15, 2024
14 of 15 checks passed
@guitargeek guitargeek deleted the roodataset branch January 15, 2024 15:57
guitargeek added a commit to guitargeek/HiggsAnalysis-CombinedLimit that referenced this pull request Apr 17, 2024
The RooDataSet constructor that takes a weight variable name was
deprecated in ROOT 6.32:
root-project/root#14317

The general command argument constructor should be used, which doesn't
change the functionality and is also backwards compatible with older
ROOT versions.

Also, avoid a unused variable warning.
guitargeek added a commit to guitargeek/HiggsAnalysis-CombinedLimit that referenced this pull request Apr 18, 2024
The RooDataSet constructor that takes a weight variable name was
deprecated in ROOT 6.32:
root-project/root#14317

The general command argument constructor should be used, which doesn't
change the functionality and is also backwards compatible with older
ROOT versions.

Also, avoid a unused variable warning.
kcormi added a commit to kcormi/HiggsAnalysis-CombinedLimit that referenced this pull request Jun 24, 2024
The RooDataSet constructor that takes a weight variable name was
deprecated in ROOT 6.32:
root-project/root#14317

The general command argument constructor should be used, which doesn't
change the functionality and is also backwards compatible with older
ROOT versions.

Also, avoid a unused variable warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants