[Python] Unit tests fail with Python debug builds

[guitargeek]

Description

After #19222 and #19223, which generally make ROOT work with debug versions of the Python interpreter, there are still unit tests failing:

The following tests FAILED:
	 10 - pyunittests-bindings-pyroot-cppyy-cppyy-test-fragile (Failed) python python_runtime_deps
	 19 - pyunittests-bindings-pyroot-cppyy-cppyy-test-templates (Failed) python python_runtime_deps
	 20 - pyunittests-bindings-pyroot-cppyy-cppyy-test-concurrent (Failed) python python_runtime_deps
	 22 - pyunittests-bindings-pyroot-cppyy-cppyy-test-stltypes (Failed) python python_runtime_deps

The following tests FAILED:
	  4 - pyunittests-bindings-pyroot-pythonizations-pyroot-pyz-pretty-printing (Failed) python
	 47 - pyunittests-bindings-pyroot-pythonizations-pyroot-pyz-rvec (Failed) python

The following tests FAILED:
	110 - roottest-python-regression-regression (Failed)    longtest

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.

We should probably prioritize the failures in the pyroot-pythonizations tests, because the other failures might to away with the new cppyy based on libInterOp anyway.

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