FindObject for rooabscollection by will-cern · Pull Request #8177 · root-project/root

will-cern · 2021-05-14T09:13:06Z

Another little change, this makes some nice features available in python such as "var" in collection

phsft-bot · 2021-05-14T09:13:08Z

Can one of the admins verify this patch?

pcanal · 2021-05-14T15:08:01Z

Can you rebase on the tip of the master branch? It seems some of the commit should already be there. Thanks.

will-cern · 2021-05-14T15:11:40Z

Can you rebase on the tip of the master branch? It seems some of the commit should already be there. Thanks.

done

pcanal · 2021-05-14T15:20:41Z

@phsft-bot build

phsft-bot · 2021-05-14T15:20:48Z

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

phsft-bot · 2021-05-14T15:39:46Z

Build failed on ROOT-fedora31/noimt.
Running on root-fedora-31-2.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Warnings:

[2021-05-14T15:26:20.062Z] /home/sftnight/build/workspace/root-pullrequests-build/build/include/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:26:20.062Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:26:24.756Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:26:24.756Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:26:25.107Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:26:25.107Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:26:25.830Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:26:25.830Z] sCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:26:26.111Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:26:26.112Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]

And 728 more

phsft-bot · 2021-05-14T15:44:31Z

Build failed on ROOT-fedora30/cxx14.
Running on root-fedora30-2.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Warnings:

[2021-05-14T15:25:24.972Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:25:24.972Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:25:24.972Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:25:24.972Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:25:26.535Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:25:26.535Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:25:26.871Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:25:26.871Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:25:27.163Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:25:27.163Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]

And 728 more

phsft-bot · 2021-05-14T15:46:00Z

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

Warnings:

[2021-05-14T15:31:01.646Z] /data/sftnight/workspace/root-pullrequests-build/build/include/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:01.646Z] /data/sftnight/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:31:31.709Z] /data/sftnight/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:31.709Z] /data/sftnight/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:31:31.709Z] /data/sftnight/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:31.709Z] /data/sftnight/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:31:31.709Z] /data/sftnight/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:31.709Z] /data/sftnight/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:31:31.998Z] /data/sftnight/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:31.998Z] /data/sftnight/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]

And 728 more

will-cern · 2021-05-14T15:48:53Z

I take it from the warnings that I might have to add the other FindObject method (that accepts a TObject pointer) to satisfy the builds? Please confirm and I can do this.

pcanal · 2021-05-14T15:59:04Z

You are correct.

hageboeck · 2021-05-14T15:59:50Z

I take it from the warnings that I might have to add the other FindObject method (that accepts a TObject pointer) to satisfy the builds? Please confirm and I can do this.

Exactly. Something like auto ptr = dynamic_cast<const RooAbsArg*>; return ptr ? find(*ptr) : nullptr; should do it.

Could you include a one-line doxygen-readable comment (///) to avoid having two undocumented functions? Also note that find(RooAbsArg&) will find objects with the same name, not necessarily with the same pointer. You should decide with @guitargeek if that's actually desired when called from the Python side. In a second sentence, you should document whether that function will find objects with the same name (= equivalent objects as far as RooFit is concerned) or if it should only find the exact same object.

phsft-bot · 2021-05-14T16:00:35Z

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

Warnings:

[2021-05-14T15:31:28.347Z] /mnt/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:21: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:28.347Z] /mnt/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:31:28.866Z] /mnt/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:21: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:28.866Z] /mnt/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:31:31.374Z] /mnt/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:21: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:31.374Z] /mnt/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:31:31.632Z] /mnt/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:21: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:31.632Z] /mnt/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:31:31.632Z] /mnt/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:21: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:31:31.632Z] /mnt/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]

And 728 more

phsft-bot · 2021-05-14T16:07:28Z

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

Warnings:

[2021-05-14T15:39:05.407Z] /home/sftnight/build/workspace/root-pullrequests-build/build/include/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:39:05.407Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:39:16.181Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:39:16.181Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:39:17.373Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:39:17.373Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:39:17.979Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:39:17.979Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]
[2021-05-14T15:39:19.612Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/base/inc/TObject.h:130:24: warning: ‘virtual TObject* TObject::FindObject(const TObject*) const’ was hidden [-Woverloaded-virtual]
[2021-05-14T15:39:19.612Z] /home/sftnight/build/workspace/root-pullrequests-build/root/roofit/roofitcore/inc/RooAbsCollection.h:114:12: warning: by ‘virtual TObject* RooAbsCollection::FindObject(const char*) const’ [-Woverloaded-virtual]

And 724 more

will-cern · 2021-05-14T16:08:51Z

I take it from the warnings that I might have to add the other FindObject method (that accepts a TObject pointer) to satisfy the builds? Please confirm and I can do this.

Exactly. Something like auto ptr = dynamic_cast<const RooAbsArg*>; return ptr ? find(*ptr) : nullptr; should do it.

Could you include a one-line doxygen-readable comment (///) to avoid having two undocumented functions? Also note that find(RooAbsArg&) will find objects with the same name, not necessarily with the same pointer. You should decide with @guitargeek if that's actually desired when called from the Python side. In a second sentence, you should document whether that function will find objects with the same name (= equivalent objects as far as RooFit is concerned) or if it should only find the exact same object.

Indeed I thought about having auto arg = dynamic_cast<const RooAbsArg*>(obj); return (arg && containsInstance(*arg)) ? obj : nullptr; but given the 'find' method matches by name I think for consistency the FindObject should behave the same for these classes, unless there is some additional benefit to FindObject(obj) returning obj??

phsft-bot · 2021-05-14T19:21:19Z

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

Errors:

[2021-05-14T19:21:18.683Z] CMake Error at C:/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1069 (ctest_start):

guitargeek

Thank you Will! I also agree that the behavior should be consistent with RooAbsCollection::find(), because the var in coll pattern is basically a pythonization of coll.find(var).

However, I think it's important to ensure this consistency in a pythonization unit test, but I'll take care of this.

This tests the functionality recently introduced in root-project#8177 (commits 3819e27 and d8cd26a).

guitargeek · 2021-05-16T22:38:13Z

Thanks again for the contributions! I implemented a unit test in this PR: #8179.

This tests the functionality recently introduced in root-project#8177 (commits 3819e27 and d8cd26a).

This tests the functionality recently introduced in #8177 (commits 3819e27 and d8cd26a).

This tests the functionality recently introduced in root-project#8177 (commits 3819e27 and d8cd26a).

will-cern requested review from Axel-Naumann, guitargeek, hageboeck, lmoneta, oshadura and pcanal as code owners May 14, 2021 09:13

Adding FindObject opens up python functionality

05159b7

will-cern force-pushed the FindObject_for_rooabscollection branch from c7248db to 05159b7 Compare May 14, 2021 15:11

pcanal approved these changes May 14, 2021

View reviewed changes

Added other FindObject signature

c4c692b

guitargeek added the in:RooFit label May 16, 2021

guitargeek approved these changes May 16, 2021

View reviewed changes

guitargeek merged commit d8cd26a into root-project:master May 16, 2021

guitargeek added a commit to guitargeek/root that referenced this pull request May 16, 2021

[RF] Add unit test for RooAbsCollection.__contains__

e3a098c

This tests the functionality recently introduced in root-project#8177 (commits 3819e27 and d8cd26a).

guitargeek mentioned this pull request May 16, 2021

[RF] More pythonizations for RooAbsCollection and child classes (RooArgList and RooArgSet) #8179

Merged

guitargeek added a commit to guitargeek/root that referenced this pull request May 16, 2021

[RF] Add unit test for RooAbsCollection.__contains__

21fbd34

This tests the functionality recently introduced in root-project#8177 (commits 3819e27 and d8cd26a).

guitargeek added a commit that referenced this pull request May 18, 2021

[RF] Add unit test for RooAbsCollection.__contains__

5ee3734

This tests the functionality recently introduced in #8177 (commits 3819e27 and d8cd26a).

pzhristov pushed a commit to alisw/root that referenced this pull request Aug 27, 2021

[RF] Add unit test for RooAbsCollection.__contains__

1052700

This tests the functionality recently introduced in root-project#8177 (commits 3819e27 and d8cd26a).

Conversation

will-cern commented May 14, 2021

Uh oh!

phsft-bot commented May 14, 2021

Uh oh!

pcanal commented May 14, 2021

Uh oh!

will-cern commented May 14, 2021

Uh oh!

pcanal commented May 14, 2021

Uh oh!

phsft-bot commented May 14, 2021

Uh oh!

phsft-bot commented May 14, 2021

Warnings:

Uh oh!

phsft-bot commented May 14, 2021

Warnings:

Uh oh!

phsft-bot commented May 14, 2021

Warnings:

Uh oh!

will-cern commented May 14, 2021

Uh oh!

pcanal commented May 14, 2021

Uh oh!

hageboeck commented May 14, 2021

Uh oh!

phsft-bot commented May 14, 2021

Warnings:

Uh oh!

phsft-bot commented May 14, 2021

Warnings:

Uh oh!

will-cern commented May 14, 2021

Uh oh!

phsft-bot commented May 14, 2021

Errors:

Uh oh!

guitargeek left a comment

Choose a reason for hiding this comment

Uh oh!

guitargeek commented May 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants