[Fix #6981] RDisplay::Print to show more elements from collections by ikabadzhov · Pull Request #9095 · root-project/root

ikabadzhov · 2021-10-11T09:08:20Z

This Pull request:

[Fix #6981] RDisplay::Print to show more elements from collections

Changes or fixes:

AddCollectionToRow() modified so that the first 10 elements of long collections
are to be printed, then followed by "...". Previously, only the first and last
elements of collections of size >=3 were printed.

Print() logic is now simplified, since after "..." there is no guarantee
that there is going to be a next element.

Current unit tests were adapted and a new one is added, to test showing
elements around the treshold, i.e. of <10, 10, 11, >11 elements.

Regarding #6981 - to display the whole collection, please use AsString().

Checklist:

tested changes locally
updated the docs (if necessary)

This PR fixes #6981

…llections AddCollectionToRow() modified so that the first 10 elements of long collections are to be printed, then followed by "...". Previously, only the first and last elements of collections of size >=3 were printed. Print() logic is now simplified, since after "..." there is no guarantee that there is going to be a next element. Current unitt tests were adapted and a new one is added, to test showing elements around the treshold, i.e. <10, 10, 11, >11. Regarding root-project#6981 - to display the whole collection, please use AsString().

phsft-bot · 2021-10-11T09:08:26Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

eguiraud

Looks good, great job!

Should we expose nMaxCollectionElements as a parameter of Display? (with a default value of 10 or 16?)

Also before merging please give this commit a clang-format pass. It should be something like git clang-format HEAD to only format the changes from your commit and not the whole files.

EDIT: from the CI failure it also looks like there is some adjustments still to do in the tests

eguiraud · 2021-10-11T09:25:16Z

+  std::vector<int> v0 (0);
+  std::vector<int> v1 (1, 0);
+  std::vector<int> v3 (3, 0);
+  std::vector<int> v9 (9, 0);
+  std::vector<int> v10 (10, 0);
+  std::vector<int> v11 (11, 0);
+  std::vector<int> v20 (20, 0);


slightly better imho (not that it changes anything in practice):

Suggested change

std::vector<int> v0 (0);

std::vector<int> v1 (1, 0);

std::vector<int> v3 (3, 0);

std::vector<int> v9 (9, 0);

std::vector<int> v10 (10, 0);

std::vector<int> v11 (11, 0);

std::vector<int> v20 (20, 0);

const std::vector<int> v0;

const std::vector<int> v1 (1);

const std::vector<int> v3 (3);

const std::vector<int> v9 (9);

const std::vector<int> v10 (10);

const std::vector<int> v11 (11);

const std::vector<int> v20 (20);

Thanks. I have just fixed the vector creation in the tests and before committing used clang-format. To me, keeping nMaxCollectionElements local to the AddCollectionToRow() makes sense since it is the only method that needs this constant.

Locally ctest -R "(dataframe|datasource)" -- -j4 passes all (121) tests.

To me, keeping nMaxCollectionElements local to the AddCollectionToRow() makes sense since it is the only method that needs this constant.

it's to let users pick a higher value if they want to

Locally ctest -R "(dataframe|datasource)" -- -j4 passes all (121) tests.

ok, let me see if I can reproduce the jenkins failures locally

fails for me locally (and it does not look like something that can depend on the platform). Make sure you don't have uncommitted changes (e.g. with git stash), rebuild and try running ./path/to/build/tree/dataframe/test/dataframe_display

phsft-bot · 2021-10-11T09:36:55Z

Build failed on mac1014/python3.
Running on macphsft17.dyndns.cern.ch:/build/jenkins/workspace/root-pullrequests-build
See console output.

Failing tests:

projectroot.tree.dataframe.test.gtest_tree_dataframe_test_dataframe_display

phsft-bot · 2021-10-11T09:47:33Z

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

projectroot.tree.dataframe.test.gtest_tree_dataframe_test_dataframe_display

phsft-bot · 2021-10-11T09:47:41Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2021-10-11T10:19:58Z

Build failed on mac1014/python3.
Running on macphsft17.dyndns.cern.ch:/build/jenkins/workspace/root-pullrequests-build
See console output.

Failing tests:

projectroot.tree.dataframe.test.gtest_tree_dataframe_test_dataframe_display

phsft-bot · 2021-10-11T10:23:50Z

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

projectroot.tree.dataframe.test.gtest_tree_dataframe_test_dataframe_display

phsft-bot · 2021-10-11T10:58:14Z

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

Failing tests:

projectroot.tree.dataframe.test.gtest_tree_dataframe_test_dataframe_display

phsft-bot · 2021-10-11T12:07:54Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2021-10-11T14:13:03Z

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

phsft-bot · 2021-10-11T16:06:36Z

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

Failing tests:

phsft-bot · 2021-10-11T17:26:20Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2021-10-11T17:27:55Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2021-10-11T17:30:53Z

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-1.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-10-11T17:30:30.336Z] FAILED: cd /mnt/build/workspace/root-pullrequests-build/build/builtins/xrootd/XROOTD-prefix/src && /usr/bin/cmake -P /mnt/build/workspace/root-pullrequests-build/build/builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download-Release.cmake && /usr/bin/cmake -E touch /mnt/build/workspace/root-pullrequests-build/build/builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download
[2021-10-11T17:30:30.336Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-11T17:31:08Z

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

Errors:

[2021-10-11T17:30:47.519Z] FAILED: builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download
[2021-10-11T17:30:47.519Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-11T17:31:58Z

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

Errors:

[2021-10-11T17:30:09.719Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-11T17:35:34Z

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-10-11T17:35:26.978Z] FAILED: builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download
[2021-10-11T17:35:26.978Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-11T17:37:43Z

Build failed on mac1014/python3.
Running on macitois21.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-10-11T17:37:33.184Z] FAILED: builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download
[2021-10-11T17:37:33.184Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-11T17:58:27Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-10-11T17:58:15.586Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

eguiraud · 2021-10-12T07:51:23Z

@phsft-bot build please

phsft-bot · 2021-10-12T07:51:28Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2021-10-12T07:54:09Z

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

Errors:

[2021-10-12T07:53:34.524Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-12T07:54:18Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-10-12T07:54:16.551Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-12T07:54:41Z

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

Errors:

[2021-10-12T07:54:39.838Z] FAILED: builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download
[2021-10-12T07:54:39.838Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-12T07:55:15Z

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-1.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-10-12T07:54:47.642Z] FAILED: cd /mnt/build/workspace/root-pullrequests-build/build/builtins/xrootd/XROOTD-prefix/src && /usr/bin/cmake -P /mnt/build/workspace/root-pullrequests-build/build/builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download-Release.cmake && /usr/bin/cmake -E touch /mnt/build/workspace/root-pullrequests-build/build/builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download
[2021-10-12T07:54:47.642Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-12T07:59:17Z

Build failed on mac11.0/cxx17.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-10-12T07:59:07.287Z] FAILED: builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download
[2021-10-12T07:59:07.287Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

phsft-bot · 2021-10-12T08:01:58Z

Build failed on mac1014/python3.
Running on macitois22.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-10-12T08:01:39.528Z] FAILED: builtins/xrootd/XROOTD-prefix/src/XROOTD-stamp/XROOTD-download
[2021-10-12T08:01:39.528Z] CMake Error at XROOTD-stamp/XROOTD-download-Release.cmake:49 (message):

eguiraud · 2021-10-13T14:01:37Z

superseded by #9105

ikabadzhov requested a review from eguiraud as a code owner October 11, 2021 09:08

eguiraud assigned ikabadzhov Oct 11, 2021

eguiraud approved these changes Oct 11, 2021

View reviewed changes

Vector creation in tests changed

cd5d59c

Print() method adjusted

c082ccd

Tests adjusted; nMaxCollectionElements changed to a property

49a2aed

Typo in importing

06ba27e

eguiraud closed this Oct 13, 2021

Uh oh!

Conversation

ikabadzhov commented Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This Pull request:

Changes or fixes:

Checklist:

Uh oh!

phsft-bot commented Oct 11, 2021

Uh oh!

eguiraud left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eguiraud Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ikabadzhov Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

eguiraud Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

eguiraud Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phsft-bot commented Oct 11, 2021

Failing tests:

Uh oh!

phsft-bot commented Oct 11, 2021

Failing tests:

Uh oh!

phsft-bot commented Oct 11, 2021

Uh oh!

phsft-bot commented Oct 11, 2021

Failing tests:

Uh oh!

phsft-bot commented Oct 11, 2021

Failing tests:

Uh oh!

phsft-bot commented Oct 11, 2021

Failing tests:

Uh oh!

phsft-bot commented Oct 11, 2021

Uh oh!

phsft-bot commented Oct 11, 2021

Failing tests:

Uh oh!

phsft-bot commented Oct 11, 2021

Failing tests:

Uh oh!

phsft-bot commented Oct 11, 2021

Uh oh!

phsft-bot commented Oct 11, 2021

Uh oh!

phsft-bot commented Oct 11, 2021

Errors:

Uh oh!

phsft-bot commented Oct 11, 2021

Errors:

Uh oh!

phsft-bot commented Oct 11, 2021

Errors:

Uh oh!

phsft-bot commented Oct 11, 2021

Errors:

Uh oh!

phsft-bot commented Oct 11, 2021

Errors:

Uh oh!

phsft-bot commented Oct 11, 2021

Errors:

Uh oh!

eguiraud commented Oct 12, 2021

Uh oh!

phsft-bot commented Oct 12, 2021

Uh oh!

phsft-bot commented Oct 12, 2021

ikabadzhov commented Oct 11, 2021 •

edited

Loading

eguiraud left a comment •

edited

Loading

eguiraud Oct 11, 2021 •

edited

Loading

eguiraud Oct 11, 2021 •

edited

Loading