Replace RooRealVar::removeRange() usage#1243
Replace RooRealVar::removeRange() usage#1243guitargeek wants to merge 1 commit intocms-analysis:mainfrom
RooRealVar::removeRange() usage#1243Conversation
ROOT has deprecated the `RooRealVar::removeRange()` method, because the name was confusing: root-project/root#21548 Instead, one should explicitly call `removeMin()` and `removeMax()`.
📝 WalkthroughWalkthroughThis pull request refactors parameter range removal across physics model and utility code files. Instead of calling Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Combine.cc`:
- Line 510: Run git-clang-format on the source file containing the Combine
implementation (the file with the comment about removeMin() and removeMax());
specifically format the file named Combine.cc so that the clang-format CI job
passes, commit the resulting changes, and push them; this will correct the style
issues flagged for symbols like removeMin and removeMax.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99aa0729-3387-4df9-8bbb-f6c5214184b0
📒 Files selected for processing (10)
docs/model_building_tutorial2024/model_building_exercise.mdpython/DegenerateMatrixRank.pypython/HiggsJPC.pypython/ModelTools.pypython/PhysicsModel.pypython/ShapeTools.pypython/TagAndProbeModel.pypython/TwoHiggsModels.pysrc/Combine.ccsrc/utils.cc
| } | ||
| // look for parameters ranged [-1e+30, 1e+30], corresponding to the old definition of unlimited parameters, | ||
| // since ROOT v6.30 have to removeRange() to keep them unlimited | ||
| // since ROOT v6.30 have to removeMin() and removeMax() to keep them unlimited |
There was a problem hiding this comment.
Run git-clang-format on this file before merge.
The clang-format job is already failing on src/Combine.cc, so this comment-only hunk is still blocking CI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Combine.cc` at line 510, Run git-clang-format on the source file
containing the Combine implementation (the file with the comment about
removeMin() and removeMax()); specifically format the file named Combine.cc so
that the clang-format CI job passes, commit the resulting changes, and push
them; this will correct the style issues flagged for symbols like removeMin and
removeMax.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (50.00%) is below the target coverage (98.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1243 +/- ##
=======================================
Coverage 20.89% 20.89%
=======================================
Files 195 195
Lines 26310 26310
Branches 3947 3947
=======================================
Hits 5498 5498
Misses 20812 20812
🚀 New features to boost your workflow:
|
1 participant
ROOT has deprecated the
RooRealVar::removeRange()method, because the name was confusing:root-project/root#21548
Instead, one should explicitly call
removeMin()andremoveMax().Summary by CodeRabbit
Bug Fixes
Documentation