[RF] More pythonizations for RooAbsCollection and child classes (RooArgList and RooArgSet) by guitargeek · Pull Request #8179 · root-project/root

guitargeek · 2021-05-16T22:37:26Z

A pythonization of RooAbsCollection::addOwned is introduced to fix issue [RF] Implement a Pythonisation for RooAbsCollection.addOwned() #8120.
The unit tests for the pythonization of RooAbsCollection were also extended to cover new developments from PR FindObject for rooabscollection #8177.
A pythonization of RooAbsCollection::operator() is introduced as discussed in fix handling of case where idx is negative #8174.

phsft-bot · 2021-05-16T22:37:34Z

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

ghost · 2021-05-16T22:48:41Z

DeepCode's analysis on #de2dd7 found:

ℹ️ 2 minor issues. 👇

Top issues

Description	Example fixes
assertTrue cannot provide an informative message about the values in the comparison _ if it fails Occurrences: rooabscollection.py:37 rooabscollection.py:64 rooabscollection.py:65	🔧 Example fixes
The call to next should be guarded with a try/except block Occurrences: rooabscollection.py:47 rooabscollection.py:48 rooabscollection.py:51	🔧 Example fixes

👉 View analysis in DeepCode’s Dashboard | Configure the bot

phsft-bot · 2021-05-16T22:58:52Z

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

Failing tests:

And 50 more

phsft-bot · 2021-05-16T23:00:41Z

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:

And 236 more

phsft-bot · 2021-05-16T23:04:31Z

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

Failing tests:

And 50 more

phsft-bot · 2021-05-16T23:15:06Z

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

Failing tests:

And 50 more

phsft-bot · 2021-05-16T23:22:21Z

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

Failing tests:

And 49 more

phsft-bot · 2021-05-16T23:27:27Z

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

Failing tests:

And 50 more

The new `RooAbsCollection::canBeAdded` member function is used in `RooAbsCollection::add`, `RooAbsCollection::addOwned`, and `RooAbsCollection::addClone`. Lke this, only `RooAbsCollection::canBeAdded` has to be overridden in `RooArgList` and `RooArgSet`, instead of overriding the three adding functions. This is convenient for pythonizing the adding functions, because if they are redefined in the child classes, one has to pythonize add redefinitions. After this PR, it is suficcient to pythonize only the adding functions in the base class `RooAbsCollection`.

The python wrapper object needs to release the ownership of the C++ RooAbsArgs object after it has been added to a owning RooAbsCollection. This fixes issue root-project#8120.

Also moves all unit tests of RooAbsCollection pythonizations into a single file.

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

phsft-bot · 2021-05-16T23:47:36Z

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-17T12:13:46Z

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

lgtm-com · 2021-05-17T12:44:03Z

This pull request introduces 1 alert when merging 8307d6d into fdb7d15 - view on LGTM.com

new alerts:

1 for Unused import

The bounds checking is now done in Python for cleaner error messages, and negative indexing is now supported.

phsft-bot · 2021-05-17T14:20:23Z

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

etejedor

As discussed on mattermost, ok for me!

guitargeek added the in:RooFit label May 16, 2021

guitargeek self-assigned this May 16, 2021

guitargeek requested review from etejedor, hageboeck, lmoneta and oshadura as code owners May 16, 2021 22:37

guitargeek mentioned this pull request May 16, 2021

FindObject for rooabscollection #8177

Merged

guitargeek added 4 commits May 17, 2021 01:47

[RF] Add pythonization for RooAbsCollection::AddOwned

fda881f

The python wrapper object needs to release the ownership of the C++ RooAbsArgs object after it has been added to a owning RooAbsCollection. This fixes issue root-project#8120.

[RF] Add unit test for pythonization of RooAbsCollection::addOwned

aa62ca3

Also moves all unit tests of RooAbsCollection pythonizations into a single file.

[RF] Add unit test for RooAbsCollection.__contains__

21fbd34

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

guitargeek force-pushed the roofit_pythonizations_1 branch from e3a098c to 21fbd34 Compare May 16, 2021 23:47

guitargeek linked an issue May 17, 2021 that may be closed by this pull request

[RF] Implement a Pythonisation for RooAbsCollection.addOwned() #8120

Closed

guitargeek mentioned this pull request May 17, 2021

fix handling of case where idx is negative #8174

Closed

guitargeek changed the title ~~[RF] Pythonize RooAbsCollection::addOwned and more pythonization unit tests~~ [RF] More pythonizations for RooAbsCollection and child classes (RooArgList and RooArgSet) May 17, 2021

[RF] Implement pythonization for RooAbsCollection::operator[]

de2dd71

The bounds checking is now done in Python for cleaner error messages, and negative indexing is now supported.

guitargeek force-pushed the roofit_pythonizations_1 branch from 8307d6d to de2dd71 Compare May 17, 2021 14:20

etejedor approved these changes May 18, 2021

View reviewed changes

guitargeek merged commit 3062aaa into root-project:master May 18, 2021

guitargeek deleted the roofit_pythonizations_1 branch May 18, 2021 12:44

guitargeek added the in:Python Interface label Jul 22, 2021

Conversation

guitargeek commented May 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phsft-bot commented May 16, 2021

Uh oh!

ghost commented May 16, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepCode's analysis on #de2dd7 found:

Top issues

👉 View analysis in DeepCode’s Dashboard | Configure the bot

Uh oh!

phsft-bot commented May 16, 2021

Failing tests:

Uh oh!

phsft-bot commented May 16, 2021

Failing tests:

Uh oh!

phsft-bot commented May 16, 2021

Failing tests:

Uh oh!

phsft-bot commented May 16, 2021

Failing tests:

Uh oh!

phsft-bot commented May 16, 2021

Failing tests:

Uh oh!

phsft-bot commented May 16, 2021

Failing tests:

Uh oh!

phsft-bot commented May 16, 2021

Uh oh!

phsft-bot commented May 17, 2021

Uh oh!

lgtm-com bot commented May 17, 2021

Uh oh!

phsft-bot commented May 17, 2021

Uh oh!

etejedor left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

guitargeek commented May 16, 2021 •

edited

Loading

ghost commented May 16, 2021 •

edited by ghost

Loading