[RF] Added new printout RooCmdArg to recreate constructor call

RooCmdArg is a bit of an old-style piece of code that doesn't really
work well with python. Also, when talking among statistics code
developers, it's commonplace to have to "exchange" fit arguments between
codes ("what arguments to you pass to make it converge?").

For this purpose, it's very convenient to:

  * be able to print the command arguments in a human-readable format,
    and

  * directly use these printouts to copy&paste them into some other
    piece of code to make comparison studies

The changes in this commit make this possible with little effort.

bindings/pyroot/pythonizations/test/CMakeLists.txt

@@ -138,24 +138,21 @@ endif()
 ROOT_ADD_PYUNITTEST(pyroot_pyz_tf_pycallables tf_pycallables.py)
 
 if(roofit)
-  # RooAbsCollection and subclasses pythonizations
-  if(NOT MSVC OR CMAKE_SIZEOF_VOID_P EQUAL 4 OR win_broken_tests)
-    ROOT_ADD_PYUNITTEST(pyroot_roofit_rooabscollection roofit/rooabscollection.py)
-  endif()
-  ROOT_ADD_PYUNITTEST(pyroot_roofit_rooarglist roofit/rooarglist.py)
-
-  # RooDataHist pythonisations
-  ROOT_ADD_PYUNITTEST(pyroot_roofit_roodatahist_ploton roofit/roodatahist_ploton.py)
 
-  # RooDataSet pythonisations
-  ROOT_ADD_PYUNITTEST(pyroot_roofit_roodataset roofit/roodataset.py)
-
-  # RooWorkspace pythonizations
   ROOT_ADD_PYUNITTEST(pyroot_roofit_rooabspdf_fitto roofit/rooabspdf_fitto.py)
   ROOT_ADD_PYUNITTEST(pyroot_roofit_rooabsreal_ploton roofit/rooabsreal_ploton.py)
-
+  ROOT_ADD_PYUNITTEST(pyroot_roofit_rooarglist roofit/rooarglist.py)
+  ROOT_ADD_PYUNITTEST(pyroot_roofit_roocmdarg roofit/roocmdarg.py)
+  ROOT_ADD_PYUNITTEST(pyroot_roofit_roodatahist_numpy roofit/roodatahist_numpy.py PYTHON_DEPS numpy)
+  ROOT_ADD_PYUNITTEST(pyroot_roofit_roodatahist_ploton roofit/roodatahist_ploton.py)
+  ROOT_ADD_PYUNITTEST(pyroot_roofit_roodataset roofit/roodataset.py)
+  ROOT_ADD_PYUNITTEST(pyroot_roofit_roodataset_numpy roofit/roodataset_numpy.py PYTHON_DEPS numpy)
   ROOT_ADD_PYUNITTEST(pyroot_roofit_roolinkedlist roofit/roolinkedlist.py)
 
+  if(NOT MSVC OR CMAKE_SIZEOF_VOID_P EQUAL 4 OR win_broken_tests)
+    ROOT_ADD_PYUNITTEST(pyroot_roofit_rooabscollection roofit/rooabscollection.py)
+  endif()
+
   if(NOT MSVC OR win_broken_tests)
     # Test pythonizations for the RooFitHS3 package, which is not built on Windows.
     ROOT_ADD_PYUNITTEST(pyroot_roofit_roojsonfactorywstool roofit/roojsonfactorywstool.py)
@@ -168,10 +165,6 @@ if(roofit)
     ROOT_ADD_PYUNITTEST(pyroot_roofit_rooworkspace roofit/rooworkspace.py)
   endif()
 
-  # NumPy compatibility
-  ROOT_ADD_PYUNITTEST(pyroot_roofit_roodataset_numpy roofit/roodataset_numpy.py PYTHON_DEPS numpy)
-  ROOT_ADD_PYUNITTEST(pyroot_roofit_roodatahist_numpy roofit/roodatahist_numpy.py PYTHON_DEPS numpy)
-
 endif()
 
 if (dataframe)

bindings/pyroot/pythonizations/test/roofit/roocmdarg.py

@@ -0,0 +1,83 @@
+import unittest
+
+import ROOT
+
+# Necessary inside the "eval" call
+RooArgSet = ROOT.RooArgSet
+RooCmdArg = ROOT.RooCmdArg
+
+x = ROOT.RooRealVar("x", "x", 1.0)
+y = ROOT.RooRealVar("y", "y", 2.0)
+z = ROOT.RooRealVar("z", "z", 3.0)
+
+
+def args_equal(arg_1, arg_2):
+    same = True
+
+    same &= str(arg_1.GetName()) == str(arg_2.GetName())
+    same &= str(arg_1.GetTitle()) == str(arg_2.GetTitle())
+
+    for i in range(2):
+        same &= arg_1.getInt(i) == arg_2.getInt(i)
+
+    for i in range(2):
+        same &= arg_1.getDouble(i) == arg_2.getDouble(i)
+
+    for i in range(3):
+        same &= str(arg_1.getString(i)) == str(arg_2.getString(i))
+
+    same &= arg_1.procSubArgs() == arg_2.procSubArgs()
+    same &= arg_1.prefixSubArgs() == arg_2.prefixSubArgs()
+
+    for i in range(2):
+        same &= arg_1.getObject(i) == arg_2.getObject(i)
+
+    def set_equal(set_1, set_2):
+        if set_1 == ROOT.nullptr and set_2 == ROOT.nullptr:
+            return True
+        if set_1 == ROOT.nullptr and set_2 != ROOT.nullptr:
+            return False
+        if set_1 != ROOT.nullptr and set_2 == ROOT.nullptr:
+            return False
+
+        if set_1.size() != set_2.size():
+            return False
+
+        return set_2.hasSameLayout(set_1)
+
+    for i in range(2):
+        same &= set_equal(arg_1.getSet(i), arg_2.getSet(i))
+
+    return same
+
+
+class TestRooArgList(unittest.TestCase):
+    """
+    Test for RooCmdArg pythonizations.
+    """
+
+    def test_constructor_eval(self):
+
+        set_1 = ROOT.RooArgSet(x, y)
+        set_2 = ROOT.RooArgSet(y, z)
+
+        def do_test(*args):
+            arg_1 = ROOT.RooCmdArg(*args)
+
+            # The arg should be able to recreate itself by emitting the right
+            # constructor code:
+            arg_2 = eval(arg_1.constructorCode())
+
+            self.assertTrue(args_equal(arg_1, arg_2))
+
+        nullp = ROOT.nullptr
+
+        # only fill the non-object fields:
+        do_test("Test", -1, 3, 4.2, 4.7, "hello", "world", nullp, nullp, nullp, "s3", nullp, nullp)
+
+        # RooArgSet tests:
+        do_test("Test", -1, 3, 4.2, 4.7, "hello", "world", nullp, nullp, nullp, "s3", set_1, set_2)
+
+
+if __name__ == "__main__":
+    unittest.main()

roofit/roofitcore/inc/RooCmdArg.h

@@ -39,6 +39,7 @@ class RooCmdArg : public TNamed {
             const char* s1=nullptr, const char* s2=nullptr,
             const TObject* o1=nullptr, const TObject* o2=nullptr, const RooCmdArg* ca=nullptr, const char* s3=nullptr,
             const RooArgSet* c1=nullptr, const RooArgSet* c2=nullptr) ;
+
   RooCmdArg(const RooCmdArg& other) ;
   RooCmdArg& operator=(const RooCmdArg& other) ;
   void addArg(const RooCmdArg& arg) ;
@@ -107,6 +108,8 @@ class RooCmdArg : public TNamed {
   bool procSubArgs() const { return _procSubArgs; }
   bool prefixSubArgs() const { return _prefixSubArgs; }
 
+  std::string constructorCode() const;
+
 private:
 
   static const RooCmdArg _none  ; ///< Static instance of null object

roofit/roofitcore/src/RooCmdArg.cxx

@@ -34,6 +34,10 @@ that create and fill these generic containers
 #include "Riostream.h"
 #include "RooArgSet.h"
 
+#include "RooFitImplHelpers.h"
+
+#include <array>
+#include <sstream>
 #include <string>
 #include <iostream>
 
@@ -207,13 +211,86 @@ void RooCmdArg::setSet(Int_t idx,const RooArgSet& set)
     _c[idx].add(set) ;
 }
 
+std::string RooCmdArg::constructorCode() const
+{
+   std::array<bool, 13> needs;
+   needs[0] = true;                  // name
+   needs[1] = true;                  // i1
+   needs[2] = _i[1] != 0;            // i2
+   needs[3] = _d[0] != 0;            // d1
+   needs[4] = _d[1] != 0;            // d2
+   needs[5] = !_s[0].empty();        // s1
+   needs[6] = !_s[1].empty();        // s2
+   needs[7] = _o[0];                 // o1
+   needs[8] = _o[1];                 // o2
+   needs[9] = !_argList.empty();     // ca
+   needs[10] = !_s[2].empty();       // s3
+   needs[11] = _c;                   // c1
+   needs[12] = _c && !_c[1].empty(); // c2
+
+   // figure out until which point we actually need to pass constructor
+   // arguments
+   bool b = false;
+   for (int i = needs.size() - 1; i >= 0; --i) {
+      b |= needs[i];
+      needs[i] = b;
+   }
+
+   std::stringstream ss;
+
+   // The first two arguments always need to be passed
+   ss << "RooCmdArg(\"" << GetName() << "\", " << _i[0];
+
+   if (needs[2])
+      ss << ", " << _i[1];
+   if (needs[3])
+      ss << ", " << _d[0];
+   if (needs[4])
+      ss << ", " << _d[1];
+   if (needs[5])
+      ss << ", " << (!_s[0].empty() ? "\"" + _s[0] + "\"" : "\"\"");
+   if (needs[6])
+      ss << ", " << (!_s[1].empty() ? "\"" + _s[1] + "\"" : "\"\"");
+   if (needs[7])
+      ss << ", " << (_o[0] ? "\"" + std::string(_o[0]->GetName()) + "\"" : "0");
+   if (needs[8])
+      ss << ", " << (_o[1] ? "\"" + std::string(_o[1]->GetName()) + "\"" : "0");
+   if (needs[9]) {
+      ss << ", ";
+      if (!_argList.empty()) {
+         ss << "{\n";
+         for (std::size_t i = 0; i < _argList.size(); ++i) {
+            if (auto *cmdArg = dynamic_cast<RooCmdArg *>(_argList.At(i))) {
+               ss << cmdArg->constructorCode() << "\n";
+            }
+         }
+         ss << "}\n";
+      } else {
+         ss << 0;
+      }
+   }
+   if (needs[10])
+      ss << ", " << (!_s[2].empty() ? "\"" + _s[2] + "\"" : "\"\"");
+   if (needs[11])
+      ss << ", RooArgSet(" << RooHelpers::getColonSeparatedNameString(_c[0], ',') << ")";
+   if (needs[12])
+      ss << ", RooArgSet(" << RooHelpers::getColonSeparatedNameString(_c[1], ',') << ")";
+   ss << ")";
+
+   return ss.str();
+}
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Print contents
-void RooCmdArg::Print(const char*) const {
-  std::cout << GetName()
-      << ":\ndoubles\t" << _d[0] << " " << _d[1]
-      << "\nints\t" << _i[0] << " " << _i[1]
-      << "\nstrings\t" << _s[0] << " " << _s[1] << " " << _s[2]
-      << "\nobjects\t" << _o[0] << " " << _o[1] << std::endl;
+// Print contents
+void RooCmdArg::Print(const char *opts) const
+{
+   TString o{opts};
+   if (o.Contains("v")) {
+      std::cout << constructorCode() << std::endl;
+      return;
+   }
+
+   std::cout << GetName() << ":\ndoubles\t" << _d[0] << " " << _d[1] << "\nints\t" << _i[0] << " " << _i[1]
+             << "\nstrings\t" << _s[0] << " " << _s[1] << " " << _s[2] << "\nobjects\t" << _o[0] << " " << _o[1]
+             << std::endl;
 }