Skip to content

Rdfd print adjusted#9105

Merged
eguiraud merged 2 commits intoroot-project:masterfrom
ikabadzhov:RDFD_Print_Adjusted
Oct 13, 2021
Merged

Rdfd print adjusted#9105
eguiraud merged 2 commits intoroot-project:masterfrom
ikabadzhov:RDFD_Print_Adjusted

Conversation

@ikabadzhov
Copy link
Copy Markdown
Contributor

@ikabadzhov ikabadzhov commented Oct 12, 2021

This Pull request: [Fix #6981] RDisplay::Print to list custom number of 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.

Additional paramter to the constructor of the the RDisplay() was added,
that is the nMaxCollectionElements. It determines, number of elements to be
shown from long collction. Default is 10.

Corresponding tests were adapted and additional ones were added to check
the new functionality.

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

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #6981

@ikabadzhov ikabadzhov requested a review from eguiraud as a code owner October 12, 2021 14:36
@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Oct 12, 2021

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
Copy link
Copy Markdown

phsft-bot commented Oct 12, 2021

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

Failing tests:

@eguiraud
Copy link
Copy Markdown
Contributor

eguiraud commented Oct 13, 2021

@ikabadzhov please squash the two commits into one and do a force-push of this branch, it will re-trigger the build and we can merge when it's done.

@eguiraud eguiraud mentioned this pull request Oct 13, 2021
Closed
2 tasks
eguiraud
eguiraud suggested changes Oct 13, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@eguiraud eguiraud left a comment

Choose a reason for hiding this comment

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

Only minor things, congrats on fixing an issue and simplifying the code at the same time

@ikabadzhov
[Fix root-project#6981] RDisplay::Print to list custom number of elem…
f1c4392 
…ents from collections

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.

Additional paramter to the constructor of the the RDisplay() was added,
that is the nMaxCollectionElements. It determines, number of elements to be
shown from long collction. Default is 10.

Corresponding tests were adapted and additional ones were added to check
the new functionality.

Regarding root-project#6981 - to display the whole collection, please use AsString().
@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Oct 13, 2021

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
Copy link
Copy Markdown

phsft-bot commented Oct 13, 2021

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
Copy link
Copy Markdown

phsft-bot commented Oct 13, 2021

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

Failing tests:

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Oct 13, 2021

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

Failing tests:

@eguiraud eguiraud merged commit 4de17ec into root-project:master Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@eguiraud eguiraud eguiraud approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ikabadzhov ikabadzhov

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[DF] Display should list all elements of a collection by default, like TTree::Scan

3 participants

@ikabadzhov @phsft-bot @eguiraud