Skip to content

Print value fixes and removal of the old code#76

Closed
perovic wants to merge 10 commits intoroot-project:masterfrom
perovic:PrintValueFixes
Closed

Print value fixes and removal of the old code#76
perovic wants to merge 10 commits intoroot-project:masterfrom
perovic:PrintValueFixes

Conversation

@perovic
Copy link
Copy Markdown
Contributor

@perovic perovic commented Aug 17, 2015

Print value fixes introduced based on comments after the first pull request.
Removed the old code and made various changes to make the code simpler.

Boris Perovic added 10 commits August 18, 2015 11:03
Minor fixes around printValue, based on comments after the integration.
9ac7d10
Removed old printValue_Default and printType_Default methods.
72874f5 
Using fully qualified name in new printType_New method.
Removed unused function hasViableCandidateToCall (overload resolution…
07294f8 
… and general fallback deals with this now).

Moved isAddressValid function from Value.cpp to ValuePrinter.cpp and implemented the check for void*.
Extracted printing of unpacked Values.
d43a8bd 
Minor comment fixes.
Changed using of QualifiedType.getAsString() to cling::utils::TypeNam…
61b5564 
…e::GetFullyQualifiedName(QualifiedType, ASTCtx).
Simplified printValue(const char *const val).
9d22da8
Removed old Stream functions from ValuePrinter.cpp.
b1320ff
Fixed the bug (missing internal::) from commit 279fc73.
bb09765
Fixed potential bug found by Coverity (https://coverity.cern.ch/repor…
c9fe04f 
…ts.htm#v13142/p10001).
Fixes based on cling test suite ("make test").
a86db5f
@Axel-Naumann
Copy link
Copy Markdown
Member

Axel-Naumann commented Aug 18, 2015

Merged into master - congrats & thanks!

Axel-Naumann
Axel-Naumann reviewed Aug 18, 2015
View reviewed changes
Comment thread interpreter/cling/include/cling/Interpreter/RuntimePrintValue.h
Copy link
Copy Markdown
Member

@Axel-Naumann Axel-Naumann Aug 18, 2015

Choose a reason for hiding this comment

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

str += ", "; - several occurrences.

Copy link
Copy Markdown
Contributor Author

@perovic perovic Aug 18, 2015

Choose a reason for hiding this comment

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

✓

@perovic perovic deleted the PrintValueFixes branch August 18, 2015 13:58
@ethereal-space-cadet16 ethereal-space-cadet16 mentioned this pull request May 31, 2022
Closed
guitargeek added a commit to guitargeek/root that referenced this pull request Mar 7, 2026
@guitargeek
[Python] Group RooFit pythonization tests in one file
9bbb310 
For small unit tests like the RooFit pythonization tests, the overhead
of starting the Python interpreter is dominant. That means the tests
should not be too granular. I think testing the RooFit pythonizations
with separate Python executions at the level of the different RooFit
classes was indeed too granular. Therefore, this commit suggests to have
a single RooFit pythonization unit test.

Running the unit tests before (single thread):
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
      Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection
 1/14 Test root-project#64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection .......   Passed    1.09 sec
      Start 65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto
 2/14 Test root-project#65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto ........   Passed    1.13 sec
      Start 66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton
 3/14 Test root-project#66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton ......   Passed    1.14 sec
      Start 67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist
 4/14 Test root-project#67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist .............   Passed    0.93 sec
      Start 68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg
 5/14 Test root-project#68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg ..............   Passed    0.95 sec
      Start 69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy
 6/14 Test root-project#69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy ......   Passed    1.32 sec
      Start 70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton
 7/14 Test root-project#70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton .....   Passed    1.09 sec
      Start 71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset
 8/14 Test root-project#71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset .............   Passed    1.03 sec
      Start 72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy
 9/14 Test root-project#72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy .......   Passed    1.70 sec
      Start 73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc
10/14 Test root-project#73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc ..........   Passed    1.21 sec
      Start 74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool
11/14 Test root-project#74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool ...   Passed    1.40 sec
      Start 75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist
12/14 Test root-project#75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist ..........   Passed    0.87 sec
      Start 76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous
13/14 Test root-project#76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous ........   Passed    1.09 sec
      Start 77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace
14/14 Test root-project#77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace ...........   Passed    1.21 sec

100% tests passed, 0 tests failed out of 14

Label Time Summary:
python                 =  16.16 sec*proc (14 tests)
python_runtime_deps    =   3.02 sec*proc (2 tests)

Total Test time (real) =  16.17 sec
```

With this commit:
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
    Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit
1/1 Test root-project#64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit ...   Passed    3.27 sec

100% tests passed, 0 tests failed out of 1

Label Time Summary:
python    =   3.27 sec*proc (1 test)

Total Test time (real) =   3.27 sec
```
guitargeek added a commit that referenced this pull request Mar 7, 2026
@guitargeek
[Python] Group RooFit pythonization tests in one file
2a74548 
For small unit tests like the RooFit pythonization tests, the overhead
of starting the Python interpreter is dominant. That means the tests
should not be too granular. I think testing the RooFit pythonizations
with separate Python executions at the level of the different RooFit
classes was indeed too granular. Therefore, this commit suggests to have
a single RooFit pythonization unit test.

Running the unit tests before (single thread):
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
      Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection
 1/14 Test #64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection .......   Passed    1.09 sec
      Start 65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto
 2/14 Test #65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto ........   Passed    1.13 sec
      Start 66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton
 3/14 Test #66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton ......   Passed    1.14 sec
      Start 67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist
 4/14 Test #67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist .............   Passed    0.93 sec
      Start 68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg
 5/14 Test #68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg ..............   Passed    0.95 sec
      Start 69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy
 6/14 Test #69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy ......   Passed    1.32 sec
      Start 70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton
 7/14 Test #70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton .....   Passed    1.09 sec
      Start 71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset
 8/14 Test #71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset .............   Passed    1.03 sec
      Start 72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy
 9/14 Test #72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy .......   Passed    1.70 sec
      Start 73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc
10/14 Test #73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc ..........   Passed    1.21 sec
      Start 74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool
11/14 Test #74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool ...   Passed    1.40 sec
      Start 75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist
12/14 Test #75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist ..........   Passed    0.87 sec
      Start 76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous
13/14 Test #76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous ........   Passed    1.09 sec
      Start 77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace
14/14 Test #77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace ...........   Passed    1.21 sec

100% tests passed, 0 tests failed out of 14

Label Time Summary:
python                 =  16.16 sec*proc (14 tests)
python_runtime_deps    =   3.02 sec*proc (2 tests)

Total Test time (real) =  16.17 sec
```

With this commit:
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
    Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit
1/1 Test #64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit ...   Passed    3.27 sec

100% tests passed, 0 tests failed out of 1

Label Time Summary:
python    =   3.27 sec*proc (1 test)

Total Test time (real) =   3.27 sec
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@perovic @Axel-Naumann