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.

Sign up for GitHub

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

Jump to bottom

[TMVA] RReader: allow spectators in xml file #7899

Merged
merged 3 commits into from
Jan 8, 2024
Merged

[TMVA] RReader: allow spectators in xml file #7899

merged 3 commits into from
Jan 8, 2024

Conversation

goi42
Copy link
Contributor

@goi42 goi42 commented Apr 16, 2021

fixes bug reported in the forum here

@goi42 goi42 mentioned this pull request Apr 16, 2021
Closed
@pieterdavid pieterdavid mentioned this pull request Jun 2, 2021
Merged
@pieterdavid
Copy link
Contributor

I opened goi42#2 with some fixes for this PR last week, but got no answer yet... shall I open a new PR directly here?

@goi42
Copy link
Contributor Author

goi42 commented Jun 11, 2021

Hi @pieterdavid,
Sorry, somehow your request fell through the cracks. I’ve merged it now. Thanks for doing this—I hadn’t had time to come back to this myself.

lmoneta
lmoneta reviewed Jul 21, 2021
View reviewed changes
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.

Hi,
The PR looks good to me. Before merging, it would be nice to update also the tutorial or add a test for it

@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from lmoneta Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from lmoneta Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@root-project root-project deleted a comment from phsft-bot Jan 8, 2024
@guitargeek
Copy link
Contributor

@phsft-bot build

@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

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 8, 2024

Test Results

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

Results for commit 327d402.

guitargeek
guitargeek approved these changes Jan 8, 2024
View reviewed changes
Copy link
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding support for the spectator variables!

@lmoneta already agreed with these changes, however his request to add documentation was not followed up. Still, it's worth it to also merge this development without this follow-up request, because spectator variables in the XML are quite common.

@guitargeek guitargeek merged commit 9645b3a into root-project:master Jan 8, 2024
14 of 15 checks passed
maksgraczyk pushed a commit to maksgraczyk/root that referenced this pull request Jan 12, 2024
@goi42 @pieterdavid @maksgraczyk
[TMVA] RReader: allow spectators in xml file (root-project#7899)
7bae969 
Implements missing feature reported in the forum [here](https://root-forum.cern.ch/t/rreader-and-spectator-variables/42844).

Closes root-project#7900.

---------

Co-authored-by: Pieter David <pieter.david@gmail.com>
vgvassilev pushed a commit to vgvassilev/root that referenced this pull request Feb 8, 2024
@goi42 @pieterdavid @vgvassilev
[TMVA] RReader: allow spectators in xml file (root-project#7899)
6176426 
Implements missing feature reported in the forum [here](https://root-forum.cern.ch/t/rreader-and-spectator-variables/42844).

Closes root-project#7900.

---------

Co-authored-by: Pieter David <pieter.david@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmoneta lmoneta lmoneta left review comments

@guitargeek guitargeek guitargeek approved these changes

@ashlaban ashlaban Awaiting requested review from ashlaban

@sitongan sitongan Awaiting requested review from sitongan

@stwunsch stwunsch Awaiting requested review from stwunsch

Assignees

@lmoneta lmoneta

Labels
in:TMVA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@goi42 @pieterdavid @guitargeek @phsft-bot @lmoneta