[Python] Unit tests fail with Python debug builds

[guitargeek]

Description

After the many PRs we have already integrated to make the tests with Python debug builds pass, there are still some tests failing:

20:pyunittests-bindings-pyroot-cppyy-cppyy-test-datatypes
97:pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy
1402:tutorial-roofit-roofit-rf409_NumPyPandasToRooFit-py
1496:tutorial-visualisation-rcanvas-df104-py
1497:tutorial-visualisation-rcanvas-df105-py
1683:roottest-python-cling-class

I could use a hand fixing these @Vipul-Cariappa. It's a good thing to do, because often the asserts that fail are relevant for thread safety, so this is a good exercise in preparation for free threaded Python.

Once the tests pass, we can use the debug version of the Python interpreter in one of our CI configurations.

[Vipul-Cariappa]

On it.
Also, this problem persists in compiler-research's forks, too. I explicitly disable Python asserts. I will also have to confirm, if this is an issue upstream.

[Vipul-Cariappa]

Hi @guitargeek, could you try out the following patch:

diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/CPPScope.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/CPPScope.cxx
index fa8e1f3b..91941883 100644
--- a/bindings/pyroot/cppyy/CPyCppyy/src/CPPScope.cxx
+++ b/bindings/pyroot/cppyy/CPyCppyy/src/CPPScope.cxx
@@ -258,6 +258,7 @@ static PyObject* pt_new(PyTypeObject* subtype, PyObject* args, PyObject* kwds)
                 // so make it accessible (the __cpp_cross__ data member also signals that
                 // this is a cross-inheritance class)
                     PyObject* bname = CPyCppyy_PyText_FromString(Cppyy::GetBaseName(result->fCppType, 0).c_str());
+                    PyErr_Clear();
                     if (PyObject_SetAttrString((PyObject*)result, "__cpp_cross__", bname) == -1)
                         PyErr_Clear();
                     Py_DECREF(bname);
@@ -540,7 +541,7 @@ static int meta_setattro(PyObject* pyclass, PyObject* pyname, PyObject* pyval)
                 meta_getattro(pyclass, pyname);       // triggers creation
         }
     }
-
+    PyErr_Clear(); // creation might have failed
     return PyType_Type.tp_setattro(pyclass, pyname, pyval);
 }
 
diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
index 2dcfc52b..291d95e1 100644
--- a/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
+++ b/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
@@ -1820,6 +1820,7 @@ bool CPyCppyy::Pythonize(PyObject* pyclass, const std::string& name)
 
         // data with size
             Utility::AddToClass(pyclass, "__real_data", "data");
+            PyErr_Clear(); // AddToClass might have failed for data
             Utility::AddToClass(pyclass, "data", (PyCFunction)VectorData);
 
         // The addition of the __array__ utility to std::vector Python proxies causes a
diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/Utility.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/Utility.cxx
index a437264b..41df97f1 100644
--- a/bindings/pyroot/cppyy/CPyCppyy/src/Utility.cxx
+++ b/bindings/pyroot/cppyy/CPyCppyy/src/Utility.cxx
@@ -552,9 +552,9 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg,
             PyErr_Clear();
 
     // ctypes function pointer
-        PyObject* argtypes = PyObject_GetAttrString(arg, "argtypes");
-        PyObject* ret = PyObject_GetAttrString(arg, "restype");
-        if (argtypes && ret) {
+        PyObject* argtypes = nullptr;
+        PyObject* ret = nullptr;
+        if ((argtypes = PyObject_GetAttrString(arg, "argtypes")) && (ret = PyObject_GetAttrString(arg, "restype"))) {
             std::ostringstream tpn;
             PyObject* pytc = PyObject_GetAttr(ret, PyStrings::gCTypesType);
             tpn << CT2CppNameS(pytc, false)
@@ -895,7 +895,7 @@ Py_ssize_t CPyCppyy::Utility::GetBuffer(PyObject* pyobject, char tc, int size, v
     if (PyObject_CheckBuffer(pyobject)) {
         if (PySequence_Check(pyobject) && !PySequence_Size(pyobject))
             return 0;   // PyObject_GetBuffer() crashes on some platforms for some zero-sized seqeunces
-
+        PyErr_Clear();
         Py_buffer bufinfo;
         memset(&bufinfo, 0, sizeof(Py_buffer));
         if (PyObject_GetBuffer(pyobject, &bufinfo, PyBUF_FORMAT) == 0) {

It fixed all of the assertion failures except for the ones in test_concurrent.py. I cannot fix it without discussing it with Wim.

If this works, will open a PR.

[guitargeek]

Awesome! Yes, indeed that fixes the tests for me. Please go ahead and open that PR 🙂

[guitargeek]

I think we can figure out the remaining failure without bothering Wim as well:

• [cppyy] Correct management of GIL in concurrency tests #19237

[guitargeek]

Hi @Vipul-Cariappa! I have updated the list of tests that still fail in the original issue post. Can you maybe still help with this please?

[Vipul-Cariappa]

Hi @guitargeek, pyunittests-bindings-pyroot-cppyy-cppyy-test-datatypes happens to pass in my system.
Also, the following:

...
1/1 Test #20: pyunittests-bindings-pyroot-cppyy-cppyy-test-datatypes ...   Passed   20.33 sec
...
1/1 Test #97: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy ...   Passed    9.03 sec
...
1/1 Test #1317: tutorial-visualisation-rcanvas-df104-py ...   Passed   79.02 sec
...
1/1 Test #1318: tutorial-visualisation-rcanvas-df105-py ...   Passed  106.03 sec
...

Could you please re-check them.

[guitargeek]

You can see these failures in the CI log of my PR that suggests to add pydebug to the CI:

#19240

Probably you need Python 3.14, like there is in Fedora Rawhide.