From 06cddc67675f1bba0db14a1486d665d1d5215b89 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 8 Oct 2026 18:47:45 +0200 Subject: [PATCH 01/12] [core] Cache the printer for each type in TCling::ToString() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TCling::ToString() forwarded to cling::Interpreter::toString(), which builds and compiles a statement calling cling::printValue() on every call. Printing many objects was therefore slow, and every call left compiled code behind (see https://github.com/root-project/root/issues/12817, which PyROOT worked around with its own cache of jitted printers). Compile a printer function on the first call for a type and reuse it. Unloading code clears the cache, because a type that is reloaded with changes needs a new printer. 🤖 Done with the help of AI --- core/metacling/src/TCling.cxx | 48 +++++++++++++++++++++++++- core/metacling/src/TCling.h | 1 + core/metacling/test/TClingTests.cxx | 53 +++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 022cc1ddba473..c134641d6cdcd 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -111,6 +111,7 @@ clang/LLVM technology. #include "cling/Interpreter/Transaction.h" #include "cling/MetaProcessor/MetaProcessor.h" #include "cling/Utils/AST.h" +#include "cling/Utils/Casting.h" #include "cling/Utils/ParserStateRAII.h" #include "cling/Utils/SourceNormalization.h" #include "cling/Interpreter/Exception.h" @@ -1056,9 +1057,50 @@ inline bool TCling::TUniqueString::Append(const std::string& str) return notPresent; } +namespace cling::valuePrinterInternal { +// Defined in cling's ValuePrinter.cpp; declares cling::printValue() the first +// time it is called. +void declarePrintValue(Interpreter &Interp); +} // namespace cling::valuePrinterInternal + +//////////////////////////////////////////////////////////////////////////////// +/// Print the object of type `type` at `obj` with cling::printValue(). +/// +/// The printer for a type is compiled on the first call and reused, so that +/// printing many objects costs no compilation, nor memory for compiled code. +/// Unloading code drops all printers (see UpdateListsOnUnloaded()), as the +/// types they were compiled for might change when reloaded. + std::string TCling::ToString(const char* type, void* obj) { - return fInterpreter->toString(type, obj); + std::string (*printer)(void *) = nullptr; + { + R__LOCKGUARD_CLING(gInterpreterMutex); + auto found = fValuePrinters.find(type); + if (found != fValuePrinters.end()) { + printer = found->second; + } else { + cling::valuePrinterInternal::declarePrintValue(*fInterpreter); + std::string name; + fInterpreter->createUniqueName(name); + name += "_ToString"; + std::string code = "std::string " + name + "(void *obj) { return cling::printValue((" + type + " *)obj); }"; + if (fInterpreter->declare(code) == cling::Interpreter::kSuccess) { + cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl()); + if (auto fd = llvm::dyn_cast_or_null( + cling::utils::Lookup::Named(&fInterpreter->getSema(), name))) { + printer = cling::utils::VoidToFunctionPtr( + fInterpreter->getAddressOfGlobal(clang::GlobalDecl(fd))); + } + } + if (!printer) { + Error("ToString", "cannot print an object of type %s", type); + return ""; + } + fValuePrinters[type] = printer; + } + } + return printer(obj); } //////////////////////////////////////////////////////////////////////////////// @@ -7079,6 +7121,10 @@ void TCling::UpdateListsOnUnloaded(const cling::Transaction &T) { HandleNewTransaction(T); + // The unloaded code might contain a type that ToString() has a printer + // for, or the printer itself + fValuePrinters.clear(); + auto Lists = std::make_tuple((TListOfDataMembers *)gROOT->GetListOfGlobals(), (TListOfFunctions *)gROOT->GetListOfGlobalFunctions(), (TListOfFunctionTemplates *)gROOT->GetListOfFunctionTemplates(), diff --git a/core/metacling/src/TCling.h b/core/metacling/src/TCling.h index 1c2be1bace445..9818535103417 100644 --- a/core/metacling/src/TCling.h +++ b/core/metacling/src/TCling.h @@ -125,6 +125,7 @@ class TCling final : public TInterpreter { std::set fAutoLoadedLibraries; // Set of libraries that were autoloaded std::hash fStringHashFunction; // A simple hashing function std::unordered_set fNSFromRootmaps; // Collection of namespaces fwd declared in the rootmaps + std::unordered_map fValuePrinters; // ToString() printer for each type name TObjArray* fRootmapFiles; // Loaded rootmap files. Bool_t fLockProcessLine; // True if ProcessLine should lock gInterpreterMutex. Bool_t fCxxModulesEnabled;// True if C++ modules was enabled diff --git a/core/metacling/test/TClingTests.cxx b/core/metacling/test/TClingTests.cxx index 08f846ee5f256..d34d4a41552d6 100644 --- a/core/metacling/test/TClingTests.cxx +++ b/core/metacling/test/TClingTests.cxx @@ -7,6 +7,8 @@ #include "gmock/gmock.h" +#include +#include #include #include #include @@ -493,6 +495,57 @@ TEST_F(TClingTests, DeclareReportsFailedStaticInit) EXPECT_EQ(2L, gInterpreter->ProcessLine("TClingTests_v;")); } +TEST_F(TClingTests, ToString) +{ + std::vector v{1, 2}; + EXPECT_EQ(gInterpreter->ToString("std::vector", &v), "{ 1, 2 }"); + // The second call reuses the printer compiled by the first + v.push_back(3); + EXPECT_EQ(gInterpreter->ToString("std::vector", &v), "{ 1, 2, 3 }"); + + int i = 42; + EXPECT_EQ(gInterpreter->ToString("int", &i), "42"); + + ROOT::TestSupport::CheckDiagsRAII diags; + diags.requiredDiag(kError, "cling", "error: expected expression", /*matchFullMessage=*/false); + diags.requiredDiag(kError, "TInterpreter::ToString", "cannot print an object of type TClingTests_NoSuchType"); + EXPECT_EQ(gInterpreter->ToString("TClingTests_NoSuchType", &i), ""); +} + +// A type that is unloaded and reloaded with changes gets a new printer. +TEST_F(TClingTests, ToStringAfterReload) +{ + // Each version has a header of its own: cling cannot reload a changed file + // after unloading it, it trips over the diagnostic state that clang keeps + // for the old contents of that file. + auto loadVersion = [](const char *header, const char *members, const char *label) { + std::ofstream(header) << "#include \n" + << "struct TClingTests_Reloaded { " << members << " };\n" + << "namespace cling {\n" + << "std::string printValue(TClingTests_Reloaded *p)\n" + << "{ return \"" << label << " \" + std::to_string(p->fX); }\n" + << "}\n"; + gInterpreter->ProcessLine((std::string(".L ") + header).c_str()); + }; + + const char *header1 = "TClingTests_ToStringAfterReload1.h"; + loadVersion(header1, "int fX = 1;", "v1"); + EXPECT_EQ(gInterpreter->ProcessLine("auto *TClingTests_r1 = new TClingTests_Reloaded;" + "gInterpreter->ToString(\"TClingTests_Reloaded\", TClingTests_r1) == \"v1 1\";"), + 1); + gInterpreter->ProcessLine((std::string(".U ") + header1).c_str()); + + const char *header2 = "TClingTests_ToStringAfterReload2.h"; + loadVersion(header2, "double fY = 0.5; int fX = 2;", "v2"); + EXPECT_EQ(gInterpreter->ProcessLine("auto *TClingTests_r2 = new TClingTests_Reloaded;" + "gInterpreter->ToString(\"TClingTests_Reloaded\", TClingTests_r2) == \"v2 2\";"), + 1); + gInterpreter->ProcessLine((std::string(".U ") + header2).c_str()); + + std::remove(header1); + std::remove(header2); +} + // https://github.com/root-project/root/issues/15818 #if !defined(_MSC_VER) || defined(R__ENABLE_BROKEN_WIN_TESTS) TEST_F(TClingTests, VeryLongExpression) From f1b0f8cf942c700965a36f3315deab09114c806a Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 8 Oct 2026 18:47:49 +0200 Subject: [PATCH 02/12] [Python] Move the pretty printing pythonization to Python MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The __str__ that prints objects with cling::printValue() was implemented in C++ in GenericPyz.cxx, with its own cache of jitted printer functions to avoid the cost of TInterpreter::ToString(). Now that ToString() caches a printer per type itself, the pythonization is a call to it. Null objects now fall back to cppyy's generic __repr__ rather than the class's own one. The std::vector pythonization defines __repr__ in terms of str(), so str() of a null std::vector proxy recursed until a RecursionError. 🤖 Done with the help of AI --- bindings/pyroot/pythonizations/CMakeLists.txt | 1 - .../python/ROOT/_pythonization/_generic.py | 25 +++- .../pyroot/pythonizations/src/GenericPyz.cxx | 114 ------------------ .../pythonizations/src/PyROOTModule.cxx | 2 - .../pythonizations/src/PyROOTPythonize.h | 2 - .../pythonizations/test/pretty_printing.py | 7 ++ 6 files changed, 29 insertions(+), 122 deletions(-) delete mode 100644 bindings/pyroot/pythonizations/src/GenericPyz.cxx diff --git a/bindings/pyroot/pythonizations/CMakeLists.txt b/bindings/pyroot/pythonizations/CMakeLists.txt index bf247a7a32def..70a847b645c07 100644 --- a/bindings/pyroot/pythonizations/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/CMakeLists.txt @@ -10,7 +10,6 @@ set(cpp_sources src/PyROOTModule.cxx - src/GenericPyz.cxx src/CPPInstancePyz.cxx ) diff --git a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_generic.py b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_generic.py index e0b1b6cf78b8e..f925da8d39614 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_generic.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_generic.py @@ -32,10 +32,29 @@ def getitem_checked(o, i): klass.__getitem__ = getitem_checked +def _cling_print_value(self): + """ + Print the object with the output of cling::printValue, like the ROOT + prompt does, falling back to __repr__ where that only gives an address. + """ + import ROOT + + if not ROOT.addressof(self): + # Null object: cppyy's generic __repr__. The class's own __repr__ may + # be defined in terms of str(), which would come back here. + return ROOT._cppyy.types.Instance.__repr__(self) + + result = ROOT.gInterpreter.ToString(type(self).__cpp_name__, self) + + if not result or result.startswith("@0x"): + # No printer, or cling only gives the address: cppyy's __repr__ says + # more + return repr(self) + return result + + # Generic pythonizor for pretty printing that is applied to (almost) all classes def pythonize_generic(klass, name): - from ROOT.libROOTPythonizations import AddPrettyPrintingPyz - # Parameters: # klass: class to be pythonized # name: string containing the name of the class @@ -57,4 +76,4 @@ def pythonize_generic(klass, name): ] if name not in exclude and not has_cpp_str: - AddPrettyPrintingPyz(klass) + klass.__str__ = _cling_print_value diff --git a/bindings/pyroot/pythonizations/src/GenericPyz.cxx b/bindings/pyroot/pythonizations/src/GenericPyz.cxx deleted file mode 100644 index e23babc97cda3..0000000000000 --- a/bindings/pyroot/pythonizations/src/GenericPyz.cxx +++ /dev/null @@ -1,114 +0,0 @@ -// Author: Stefan Wunsch, Enric Tejedor CERN 06/2018 -// Original PyROOT code by Wim Lavrijsen, LBL - -/************************************************************************* - * Copyright (C) 1995-2018, Rene Brun and Fons Rademakers. * - * All rights reserved. * - * * - * For the licensing terms see $ROOTSYS/LICENSE. * - * For the list of contributors see $ROOTSYS/README/CREDITS. * - *************************************************************************/ - -#include "Python.h" - -#include "cpyrt/API.h" - -#include "../../cppjit/src/interop/cppjit_interop.h" -#include "../../cppjit/src/cpyrt/Utility.h" - -#include "PyROOTPythonize.h" - -#include "TClass.h" -#include "TInterpreter.h" -#include "TInterpreterValue.h" - -#include - -using namespace cppjit::cpyrt; - -// We take as unique identifier the declId of the class to -// treat the case where a class is loaded, an instance printed, -// the class unloaded and reloaded with changes. -static ULong64_t GetClassID(const char *clName) -{ - if (auto cl = TClass::GetClass(clName)) { - if (auto clInfo = cl->GetClassInfo()) { - return reinterpret_cast(gInterpreter->GetDeclId(clInfo)); - } - } - return 0; -} - -PyObject *ClingPrintValue(PyObject *self, PyObject * /* args */) -{ - // Map holding the classID of the classes and the pointer - // to the printer function. - static std::map declIDPrinterMap; - - auto cppObj = cppjit::cpyrt::Instance_AsVoidPtr(self); - if (!cppObj) - // Proxied cpp object is null, use cppyy's generic __repr__ - return PyObject_Repr(self); - - // We jit the helper only once, at the first invocation of any - // printer. The integer parameter is there to make sure we have - // different instances of the printing function in presence of - // unload-reload events. - if (0 == declIDPrinterMap.size()) { - std::string printerCode = "namespace ROOT::Internal::Pythonizations::ValuePrinters" - "{" - " template std::string ValuePrinter(void *obj)" - " {" - " return cling::printValue((T *)obj);" - " }" - "}"; - gInterpreter->Declare(printerCode.c_str()); - } - - const std::string className = cppjit::cpyrt::Instance_GetScopedFinalName(self); - - std::string printResult; - - if (const auto classID = GetClassID(className.c_str())) { - // If we never encountered this class, we jit the function which - // is necessary to print it and store it in the map instantiated - // above. Otherwise, we just use the pointer to the previously - // jitted function. This allows to jit the printer only once per - // type, at the modest price of a typename and pointer stored in - // memory. - auto &printerFuncrPtr = declIDPrinterMap[classID]; - - if (!printerFuncrPtr) { - std::string printFuncName = "ROOT::Internal::Pythonizations::ValuePrinters::ValuePrinter<" + className + ", " + - std::to_string(classID) + ">"; - printerFuncrPtr = (void *)gInterpreter->Calc(printFuncName.c_str()); - } - printResult = ((std::string(*)(void *))printerFuncrPtr)(cppObj); - } else { - // If something went wrong, we use the slow method - printResult = gInterpreter->ToString(className.c_str(), cppObj); - } - - if (printResult.find("@0x") == 0) { - // Fall back to __repr__ if we just get an address from cling - return PyObject_Repr(self); - } else { - return PyUnicode_FromString(printResult.c_str()); - } -} - -//////////////////////////////////////////////////////////////////////////// -/// \brief Add pretty printing pythonization -/// \param[in] self Always null, since this is a module function. -/// \param[in] args Pointer to a Python tuple object containing the arguments -/// received from Python. -/// -/// This function adds the following pythonizations to print the object more -/// user-friendly than cppyy by using the output of cling::printValue as the -/// return value of the special method __str__. -PyObject *PyROOT::AddPrettyPrintingPyz(PyObject * /* self */, PyObject *args) -{ - PyObject *pyclass = PyTuple_GetItem(args, 0); - Utility::AddToClass(pyclass, "__str__", (PyCFunction)ClingPrintValue); - Py_RETURN_NONE; -} diff --git a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx index 6dce34a0e970e..b151fa9ddcc84 100644 --- a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx +++ b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx @@ -156,8 +156,6 @@ PyObject *InstallGUIEventInputHook(PyObject * /*self*/, PyObject * /*args*/) static PyMethodDef gPyROOTMethods[] = { {"AddCPPInstancePickling", (PyCFunction)PyROOT::AddCPPInstancePickling, METH_NOARGS, "Add a custom pickling mechanism for Cppyy Python proxy objects"}, - {"AddPrettyPrintingPyz", (PyCFunction)PyROOT::AddPrettyPrintingPyz, METH_VARARGS, - "Add pretty printing pythonization"}, {"InstallWarningHandler", (PyCFunction)InstallWarningHandler, METH_NOARGS, "Turn ROOT warnings into Python warnings"}, {"InstallGUIEventInputHook", (PyCFunction)InstallGUIEventInputHook, METH_NOARGS, diff --git a/bindings/pyroot/pythonizations/src/PyROOTPythonize.h b/bindings/pyroot/pythonizations/src/PyROOTPythonize.h index b74b00d6ae642..0c948bbd568f9 100644 --- a/bindings/pyroot/pythonizations/src/PyROOTPythonize.h +++ b/bindings/pyroot/pythonizations/src/PyROOTPythonize.h @@ -18,8 +18,6 @@ namespace PyROOT { PyObject *AddCPPInstancePickling(PyObject *self, PyObject *args); -PyObject *AddPrettyPrintingPyz(PyObject *self, PyObject *args); - PyObject *CPPInstanceExpand(PyObject *self, PyObject *args); } // namespace PyROOT diff --git a/bindings/pyroot/pythonizations/test/pretty_printing.py b/bindings/pyroot/pythonizations/test/pretty_printing.py index 266f5c5195c88..e82b606bfb6ec 100644 --- a/bindings/pyroot/pythonizations/test/pretty_printing.py +++ b/bindings/pyroot/pythonizations/test/pretty_printing.py @@ -86,6 +86,13 @@ def test_null_object(self): self.assertIn("TTree object at", s) self.assertEqual(s, r) + def test_null_object_with_str_based_repr(self): + # The std::vector pythonization defines __repr__ in terms of str(), + # so the fallback for null objects must not go through __repr__ + x = ROOT.MakeNullPointer("std::vector") + # Not "at 0x0": glibc prints a null pointer as "(nil)" + self.assertIn("vector object at", str(x)) + def test_user_class_with_str(self): # ROOT-10967: Respect existing __str__ method defined in C++ ROOT.gInterpreter.Declare('struct MyClassWithStr { std::string __str__() { return "foo"; } };') From 619bfbfe9d382eff5b12bb057599fce3370cb134 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 8 Oct 2026 18:54:44 +0200 Subject: [PATCH 03/12] [cppjit] Keep multi-word types in returned function pointer signatures [upstream] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The executor for a function pointer return type wraps the pointer in a std::function, compiled from the return type and signature it takes from the type name. CreateExecutor(TCppType_t) took them from the type name with all spaces removed for the executor lookups, so a signature with a multi-word type did not compile: std::function ... TypeError: conversion to std::function failed which broke e.g. SetErrorHandler() in ROOT, which returns the previous handler. Take them from the type as spelled instead. The new test is separate from test26_function_pointers, which does not run on macOS. 🤖 Done with the help of AI --- bindings/pyroot/cppjit/src/cpyrt/Executors.cxx | 16 ++++++++++------ bindings/pyroot/cppjit/test/test_datatypes.py | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/Executors.cxx b/bindings/pyroot/cppjit/src/cpyrt/Executors.cxx index 6fc105e417f9f..4633eac5e0198 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Executors.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Executors.cxx @@ -1093,14 +1093,18 @@ cpyrt::Executor* cpyrt::CreateExecutor(interop::TCppType_t type, cdims_t dims) { result = new InstancePtrExecutor(klass); } else if (realTypeStr.find("(*)") != std::string::npos || (realTypeStr.find("::*)") != std::string::npos)) { - // this is a function pointer + // this is a function pointer; the executor compiles the return type and + // signature, so take them from the type as spelled: realTypeStr has its + // spaces removed for the lookups above, which turns e.g. "const char" + // into "constchar" // TODO: find better way of finding the type - auto pos1 = realTypeStr.find('('); - auto pos2 = realTypeStr.find("*)"); - auto pos3 = realTypeStr.rfind(')'); + const std::string fnTypeStr = interop::GetTypeAsString(resolvedType); + auto pos1 = fnTypeStr.find('('); + auto pos2 = fnTypeStr.find("*)"); + auto pos3 = fnTypeStr.rfind(')'); result = new FunctionPointerExecutor( - realTypeStr.substr(0, pos1), - realTypeStr.substr(pos2 + 2, pos3 - pos2 - 1)); + fnTypeStr.substr(0, pos1), + fnTypeStr.substr(pos2 + 2, pos3 - pos2 - 1)); } else { // unknown: void* may work ("user knows best"), void will fail on use of // return value diff --git a/bindings/pyroot/cppjit/test/test_datatypes.py b/bindings/pyroot/cppjit/test/test_datatypes.py index 5e508d32dcee6..69abde9e8a71b 100644 --- a/bindings/pyroot/cppjit/test/test_datatypes.py +++ b/bindings/pyroot/cppjit/test/test_datatypes.py @@ -2777,6 +2777,22 @@ def test55_qt_cache_alias_collision(self): ns.take_int8(101) raises(TypeError, ns.take_int8, "e") + def test56_function_pointer_return_multiword_types(self): + """A returned function pointer whose signature has multi-word types""" + + import cppjit + + cppjit.cppdef(r"""\ + namespace ns56 { + typedef unsigned long (*func_t)(const char *, unsigned int); + unsigned long add_len(const char *s, unsigned int n) { return std::string(s).size() + n; } + func_t get() { return add_len; } + }""") + + # the std::function wrapping the returned pointer is compiled from + # its signature, which must keep "const char" and "unsigned int" + assert cppjit.gbl.ns56.get()("abc", 4) == 7 + class TestANONENUM: def test01_anonymous_enum_repeated_access(self): From 94c102d634a68ab36880bc6de29479696621fe44 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 8 Oct 2026 19:13:58 +0200 Subject: [PATCH 04/12] [Python] Move the JupyROOT output capture and executors to Python MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IOHandler.cxx captured the stdout and stderr of C and C++ code in notebooks by redirecting the file descriptors into pipes, and ran notebook cells through TInterpreter, reporting C++ exceptions. Both are now Python: the capture with os.dup2() and pipes, the cell execution through gInterpreter, with cppyy translating C++ exceptions. The capture used to read the pipes only after flushing C stdio, so a cell that wrote more than the pipe holds (64 kB on macOS) hung: the writer blocked on the full pipe holding the stream lock that the flush waited for. Now a thread per stream drains its pipe as long as the capture runs. A process started during the capture can keep a pipe open, so the end of a capture waits at most a second for the rest of the output. Other differences: - each IOHandler has its own capture, instead of all sharing one, which the first one to be deleted destroyed; - deleting an IOHandler ends its capture; - output that is not valid UTF-8 is decoded with replacement characters instead of raising; - ProcessLine() and Declare() release the GIL (set in the facade), so the ROOT kernel streams output while a cell runs. Tested by running the JupyROOT notebooks of roottest with Jupyter installed, as they are disabled in builds without it. 🤖 Done with the help of AI --- .../python/ROOT/_jupyroot/helpers/handlers.py | 164 +++++++++- .../pyroot/pythonizations/src/IOHandler.cxx | 287 ------------------ .../pythonizations/src/PyROOTModule.cxx | 2 - 3 files changed, 153 insertions(+), 300 deletions(-) delete mode 100644 bindings/pyroot/pythonizations/src/IOHandler.cxx diff --git a/bindings/pyroot/pythonizations/python/ROOT/_jupyroot/helpers/handlers.py b/bindings/pyroot/pythonizations/python/ROOT/_jupyroot/helpers/handlers.py index 3280c6ca32865..2b20140462929 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_jupyroot/helpers/handlers.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_jupyroot/helpers/handlers.py @@ -12,15 +12,97 @@ # For the list of contributors see $ROOTSYS/README/CREDITS. # ################################################################################ +import codecs import contextvars +import ctypes +import os import queue import sys +import threading +import time from threading import Thread from time import sleep as timeSleep -import ROOT.libROOTPythonizations as _lib from ROOT._jupyroot import helpers +# C stdio, whose buffers have to be flushed for output to reach the pipes +if sys.platform == "win32": + _crt = ctypes.cdll.ucrtbase +else: + _crt = ctypes.CDLL(None) +_crt.fflush.argtypes = [ctypes.c_void_p] + +# How long to wait at the end of a capture for the rest of the output. It only +# takes this long when a process started during the capture still holds the +# pipe, so that its end is never reached. +_END_CAPTURE_TIMEOUT = 1.0 + + +class _FileDescriptorCapture(object): + """Redirects a file descriptor into a pipe and collects what is written + to it, also by C and C++ code. + + A thread reads the pipe for as long as the capture runs, so that writers + never wait on a full pipe. A writer of C stdio does so holding the lock of + the stream, and flushing the stream to get its output would then wait for + that lock forever. + """ + + def __init__(self, fd): + self._fd = fd + self._saved_fd = None + self._write_fd = None + self._reader = None + self._lock = threading.Lock() + self._text = "" + + @property + def text(self): + with self._lock: + return self._text + + def clear(self): + with self._lock: + self._text = "" + + def start(self): + self._saved_fd = os.dup(self._fd) + read_fd, self._write_fd = os.pipe() + os.dup2(self._write_fd, self._fd) + self._reader = threading.Thread(target=self._read, args=(read_fd,), name="JupyROOT capture", daemon=True) + self._reader.start() + + def _read(self, read_fd): + # Decode incrementally: a read can end in the middle of a character + decoder = codecs.getincrementaldecoder("utf-8")(errors="replace") + try: + while True: + chunk = os.read(read_fd, 65536) + text = decoder.decode(chunk, final=not chunk) + with self._lock: + # After a timeout in stop(), the output belongs to no + # capture anymore + if self._reader is threading.current_thread(): + self._text += text + if not chunk: + break + finally: + os.close(read_fd) + + def stop(self): + """Restore the file descriptor; call wait() for the rest of the output""" + os.dup2(self._saved_fd, self._fd) + os.close(self._saved_fd) + os.close(self._write_fd) + self._saved_fd = self._write_fd = None + + def wait(self, deadline): + """Wait until the output written before stop() is collected, or until + the deadline (from time.monotonic()) has passed""" + self._reader.join(max(0.0, deadline - time.monotonic())) + with self._lock: + self._reader = None + class IOHandler(object): r"""Class used to capture output from C/C++ libraries. @@ -36,28 +118,48 @@ class IOHandler(object): """ def __init__(self): - _lib.JupyROOTExecutorHandler_Ctor() + import ROOT + + # Fixes for ROOT-7999 + ROOT.SetErrorHandler(ROOT.DefaultErrorHandler) + + self._stdout = _FileDescriptorCapture(1) + self._stderr = _FileDescriptorCapture(2) + self._capturing = False def __del__(self): - _lib.JupyROOTExecutorHandler_Dtor() + self.EndCapture() def Clear(self): - _lib.JupyROOTExecutorHandler_Clear() + self._stdout.clear() + self._stderr.clear() def Poll(self): - _lib.JupyROOTExecutorHandler_Poll() + if self._capturing: + # The reader threads collect what reaches the pipes + _crt.fflush(None) def InitCapture(self): - _lib.JupyROOTExecutorHandler_InitCapture() + if not self._capturing: + self._stdout.start() + self._stderr.start() + self._capturing = True def EndCapture(self): - _lib.JupyROOTExecutorHandler_EndCapture() + if self._capturing: + _crt.fflush(None) + self._stdout.stop() + self._stderr.stop() + deadline = time.monotonic() + _END_CAPTURE_TIMEOUT + self._stdout.wait(deadline) + self._stderr.wait(deadline) + self._capturing = False def GetStdout(self): - return _lib.JupyROOTExecutorHandler_GetStdout() + return self._stdout.text def GetStderr(self): - return _lib.JupyROOTExecutorHandler_GetStderr() + return self._stderr.text def GetStreamsDicts(self): out = self.GetStdout() @@ -145,6 +247,46 @@ def HasFinished(self): return not self.poller.is_running +def _report_exception(location, e): + # Report exceptions that escape the interpreted code like TRint does at + # the ROOT prompt (see TRint::HandleTermInput), instead of swallowing them + # silently (ROOT-10589) + import ROOT + + if isinstance(e, ROOT.std.exception): + message = "{} caught: {}".format(type(e).__cpp_name__, e.what()) + ROOT.Error(location, message.replace("%", "%%")) + else: + ROOT.Error(location, "Exception caught!") + + +def _jupyroot_execute(code): + import ROOT + + status = False + try: + err = ctypes.c_uint(ROOT.TInterpreter.kNoError) + if ROOT.gInterpreter.ProcessLine(code, err): + status = True + if err.value == ROOT.TInterpreter.kProcessing: + ROOT.gInterpreter.ProcessLine(".@") + ROOT.gInterpreter.ProcessLine('cerr << "Unbalanced braces. This cell was not processed." << endl;') + except Exception as e: + _report_exception("JupyROOTExecutor", e) + return int(status) + + +def _jupyroot_declare(code): + import ROOT + + status = False + try: + status = bool(ROOT.gInterpreter.Declare(code)) + except Exception as e: + _report_exception("JupyROOTDeclarer", e) + return int(status) + + class JupyROOTDeclarer(Runner): """Asynchrously execute declarations >>> import ROOT @@ -158,7 +300,7 @@ class JupyROOTDeclarer(Runner): """ def __init__(self, poller): - super(JupyROOTDeclarer, self).__init__(_lib.JupyROOTDeclarer, poller) + super(JupyROOTDeclarer, self).__init__(_jupyroot_declare, poller) class JupyROOTExecutor(Runner): @@ -172,7 +314,7 @@ class JupyROOTExecutor(Runner): """ def __init__(self, poller): - super(JupyROOTExecutor, self).__init__(_lib.JupyROOTExecutor, poller) + super(JupyROOTExecutor, self).__init__(_jupyroot_execute, poller) def display_drawables(displayFunction): diff --git a/bindings/pyroot/pythonizations/src/IOHandler.cxx b/bindings/pyroot/pythonizations/src/IOHandler.cxx deleted file mode 100644 index 8949538f06043..0000000000000 --- a/bindings/pyroot/pythonizations/src/IOHandler.cxx +++ /dev/null @@ -1,287 +0,0 @@ -// Author: Danilo Piparo, Omar Zapata, Enric Tejedor 16/12/2015 - -/************************************************************************* - * Copyright (C) 1995-2020, Rene Brun and Fons Rademakers. * - * All rights reserved. * - * * - * For the licensing terms see $ROOTSYS/LICENSE. * - * For the list of contributors see $ROOTSYS/README/CREDITS. * - *************************************************************************/ - -#include - -#include -#ifdef _MSC_VER // Visual Studio -#include -#include -#pragma comment(lib, "Ws2_32.lib") -#define pipe(fds) _pipe(fds, 1048575, _O_BINARY) -#define read _read -#define dup _dup -#define dup2 _dup2 -#define STDIN_FILENO 0 -#define STDOUT_FILENO 1 -#define STDERR_FILENO 2 -#else -#include -#endif -#include -#include -#include -#include "TClassEdit.h" -#include "TError.h" -#include "TInterpreter.h" - -////////////////////////// -// MODULE FUNCTIONALITY // -////////////////////////// - -class JupyROOTExecutorHandler { -private: - bool fCapturing = false; - std::string fStdoutpipe; - std::string fStderrpipe; - int fStdout_pipe[2] = {0, 0}; - int fStderr_pipe[2] = {0, 0}; - int fSaved_stderr = 0; - int fSaved_stdout = 0; - -public: - JupyROOTExecutorHandler(); - void Poll(); - void InitCapture(); - void EndCapture(); - void Clear(); - std::string &GetStdout(); - std::string &GetStderr(); -}; - -#ifndef F_LINUX_SPECIFIC_BASE -#define F_LINUX_SPECIFIC_BASE 1024 -#endif -#ifndef F_SETPIPE_SZ -#define F_SETPIPE_SZ (F_LINUX_SPECIFIC_BASE + 7) -#endif - -constexpr long MAX_PIPE_SIZE = 1048575; - -JupyROOTExecutorHandler::JupyROOTExecutorHandler() {} - -static void PollImpl(FILE *stdStream, int *pipeHandle, std::string &pipeContent) -{ - fflush(stdStream); -#ifdef _MSC_VER - char buffer[60000] = ""; - struct _stat st; - _fstat(pipeHandle[0], &st); - if (st.st_size) { - _read(pipeHandle[0], buffer, 60000); - pipeContent += buffer; - } -#else - int buf_read; - char ch; - while (true) { - buf_read = read(pipeHandle[0], &ch, 1); - if (buf_read == 1) { - pipeContent += ch; - } else - break; - } -#endif -} - -void JupyROOTExecutorHandler::Poll() -{ - PollImpl(stdout, fStdout_pipe, fStdoutpipe); - PollImpl(stderr, fStderr_pipe, fStderrpipe); -} - -static void InitCaptureImpl(int &savedStdStream, int *pipeHandle, int FILENO) -{ - savedStdStream = dup(FILENO); - if (pipe(pipeHandle) != 0) { - return; - } -#ifndef _MSC_VER - long flags_stdout = fcntl(pipeHandle[0], F_GETFL); - if (flags_stdout == -1) - return; - flags_stdout |= O_NONBLOCK; - fcntl(pipeHandle[0], F_SETFL, flags_stdout); - fcntl(pipeHandle[0], F_SETPIPE_SZ, MAX_PIPE_SIZE); -#endif - dup2(pipeHandle[1], FILENO); -} - -void JupyROOTExecutorHandler::InitCapture() -{ - if (!fCapturing) { - InitCaptureImpl(fSaved_stdout, fStdout_pipe, STDOUT_FILENO); - InitCaptureImpl(fSaved_stderr, fStderr_pipe, STDERR_FILENO); - fCapturing = true; - } -} - -void JupyROOTExecutorHandler::EndCapture() -{ - if (fCapturing) { - Poll(); - dup2(fSaved_stdout, STDOUT_FILENO); - dup2(fSaved_stderr, STDERR_FILENO); - close(fSaved_stdout); - close(fSaved_stderr); - close(fStdout_pipe[0]); - close(fStdout_pipe[1]); - close(fStderr_pipe[0]); - close(fStderr_pipe[1]); - fCapturing = false; - } -} - -void JupyROOTExecutorHandler::Clear() -{ - fStdoutpipe = ""; - fStderrpipe = ""; -} - -std::string &JupyROOTExecutorHandler::GetStdout() -{ - return fStdoutpipe; -} - -std::string &JupyROOTExecutorHandler::GetStderr() -{ - return fStderrpipe; -} - -JupyROOTExecutorHandler *JupyROOTExecutorHandler_ptr = nullptr; - -// Report exceptions that escape the interpreted code like TRint does at the -// ROOT prompt (see TRint::HandleTermInput), instead of swallowing them -// silently (ROOT-10589). -static void ReportCaughtException(const char *location, const std::exception &e) -{ - int err = 0; - char *demangledType_c = TClassEdit::DemangleTypeIdName(typeid(e), err); - const char *demangledType = err ? "" : demangledType_c; - ::Error(location, "%s caught: %s", demangledType, e.what()); - free(demangledType_c); -} - -bool JupyROOTExecutorImpl(const char *code) -{ - auto status = false; - try { - auto err = TInterpreter::kNoError; - if (gInterpreter->ProcessLine(code, &err)) { - status = true; - } - - if (err == TInterpreter::kProcessing) { - gInterpreter->ProcessLine(".@"); - gInterpreter->ProcessLine("cerr << \"Unbalanced braces. This cell was not processed.\" << endl;"); - } - } catch (std::exception &e) { - ReportCaughtException("JupyROOTExecutor", e); - } catch (...) { - ::Error("JupyROOTExecutor", "Exception caught!"); - } - - return status; -} - -bool JupyROOTDeclarerImpl(const char *code) -{ - auto status = false; - try { - if (gInterpreter->Declare(code)) { - status = true; - } - } catch (std::exception &e) { - ReportCaughtException("JupyROOTDeclarer", e); - } catch (...) { - ::Error("JupyROOTDeclarer", "Exception caught!"); - } - return status; -} - -PyObject *JupyROOTExecutor(PyObject * /*self*/, PyObject *args) -{ - const char *code; - if (!PyArg_ParseTuple(args, "s", &code)) - return NULL; - - auto res = JupyROOTExecutorImpl(code); - - return PyLong_FromLong(res); -} - -PyObject *JupyROOTDeclarer(PyObject * /*self*/, PyObject *args) -{ - const char *code; - if (!PyArg_ParseTuple(args, "s", &code)) - return NULL; - - auto res = JupyROOTDeclarerImpl(code); - - return PyLong_FromLong(res); -} - -PyObject *JupyROOTExecutorHandler_Clear(PyObject * /*self*/, PyObject * /*args*/) -{ - JupyROOTExecutorHandler_ptr->Clear(); - Py_RETURN_NONE; -} - -PyObject *JupyROOTExecutorHandler_Ctor(PyObject * /*self*/, PyObject * /*args*/) -{ - if (!JupyROOTExecutorHandler_ptr) { - JupyROOTExecutorHandler_ptr = new JupyROOTExecutorHandler(); - // Fixes for ROOT-7999 - gInterpreter->ProcessLine("SetErrorHandler((ErrorHandlerFunc_t)&DefaultErrorHandler);"); - } - - Py_RETURN_NONE; -} - -PyObject *JupyROOTExecutorHandler_Poll(PyObject * /*self*/, PyObject * /*args*/) -{ - JupyROOTExecutorHandler_ptr->Poll(); - Py_RETURN_NONE; -} - -PyObject *JupyROOTExecutorHandler_EndCapture(PyObject * /*self*/, PyObject * /*args*/) -{ - JupyROOTExecutorHandler_ptr->EndCapture(); - Py_RETURN_NONE; -} - -PyObject *JupyROOTExecutorHandler_InitCapture(PyObject * /*self*/, PyObject * /*args*/) -{ - JupyROOTExecutorHandler_ptr->InitCapture(); - Py_RETURN_NONE; -} - -PyObject *JupyROOTExecutorHandler_GetStdout(PyObject * /*self*/, PyObject * /*args*/) -{ - auto out = JupyROOTExecutorHandler_ptr->GetStdout().c_str(); - return PyUnicode_FromString(out); -} - -PyObject *JupyROOTExecutorHandler_GetStderr(PyObject * /*self*/, PyObject * /*args*/) -{ - auto err = JupyROOTExecutorHandler_ptr->GetStderr().c_str(); - return PyUnicode_FromString(err); -} - -PyObject *JupyROOTExecutorHandler_Dtor(PyObject * /*self*/, PyObject * /*args*/) -{ - if (!JupyROOTExecutorHandler_ptr) - Py_RETURN_NONE; - - delete JupyROOTExecutorHandler_ptr; - JupyROOTExecutorHandler_ptr = nullptr; - - Py_RETURN_NONE; -} diff --git a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx index b151fa9ddcc84..70a4f4c1fcad5 100644 --- a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx +++ b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx @@ -30,8 +30,6 @@ #include #include -#include "IOHandler.cxx" - namespace PyROOT { PyObject *gRootModule = nullptr; From 75f1b8d265e888d45adf512523a353926b593c43 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 8 Oct 2026 20:47:38 +0200 Subject: [PATCH 05/12] [cppjit] Add ll.as_std_any() to keep Python objects alive from C++ [upstream] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C++ code that reads memory owned by a Python object, like a data source over NumPy arrays, needs to keep the object alive for as long as it may read, which can be past the lifetime of any Python reference to it. ROOT had its own CPython extension function for this, wrapping a reference in a std::any. ll.as_std_any(obj) returns such a std::any. It and every copy C++ makes of it hold a reference to obj. As C++ may copy or destroy them on any thread, the reference count is changed with the GIL taken; after Python is finalized, the reference is left be. 🤖 Done with the help of AI --- bindings/pyroot/cppjit/python/cppjit/ll.py | 15 +++++ .../pyroot/cppjit/src/cpyrt/cpyrtModule.cxx | 46 +++++++++++++++ bindings/pyroot/cppjit/test/test_lowlevel.py | 57 +++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/bindings/pyroot/cppjit/python/cppjit/ll.py b/bindings/pyroot/cppjit/python/cppjit/ll.py index 4a9895eb95528..71220e25c503d 100644 --- a/bindings/pyroot/cppjit/python/cppjit/ll.py +++ b/bindings/pyroot/cppjit/python/cppjit/ll.py @@ -26,6 +26,7 @@ "array_new", "array_delete", "bind_value", + "as_std_any", "signals_as_exception", "set_signals_as_exception", "FatalError", @@ -71,6 +72,20 @@ def bind_value(type_name, address, dims=None): return cppjit._backend.bind_value(type_name, address, dims) +def as_std_any(obj): + """Wrap a reference to the Python object `obj` in a `std::any`. + + For C++ code that has to keep `obj` alive, such as a data source reading + from the memory of a NumPy array: the `std::any`, and each copy that C++ + makes of it, holds a reference to `obj`, which is released when that copy + is destroyed. The reference count is changed with the GIL taken, so C++ + may do this from any thread. + + keepalive = ll.as_std_any(array) + """ + return cppjit._backend.as_std_any(obj) + + # import low-level python converters for _name in ["addressof", "as_cobject", "as_capsule", "as_ctypes", "as_memoryview"]: try: diff --git a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx index 880b1466d8ff5..701809b496280 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx @@ -32,6 +32,7 @@ PyObject* Instance_FromVoidPtr(void* addr, const std::string& classname, // Standard #include +#include #include #include #include @@ -666,6 +667,49 @@ static PyObject* BindObject(PyObject*, PyObject* args, PyObject* kwds) { return BindCppObjectNoCast(addr, cast_type); } +//---------------------------------------------------------------------------- +namespace { +// A reference to a Python object, held by C++. Copies hold references of +// their own. C++ may copy or destroy it on any thread, so it takes the GIL to +// change the reference count; once Python is finalized it can't, and leaves +// the reference be. +class PyObjectRef { + PyObject* fObject; + +public: + // called from Python, with the GIL held + explicit PyObjectRef(PyObject* object) : fObject(object) { + Py_INCREF(fObject); + } + PyObjectRef(const PyObjectRef& other) : fObject(other.fObject) { + if (Py_IsInitialized()) { + cpyrt::PythonGILRAII python_gil_raii; + Py_INCREF(fObject); + } + } + PyObjectRef(PyObjectRef&& other) noexcept : fObject(other.fObject) { + other.fObject = nullptr; + } + PyObjectRef& operator=(PyObjectRef other) noexcept { + std::swap(fObject, other.fObject); + return *this; + } + ~PyObjectRef() { + if (fObject && Py_IsInitialized()) { + cpyrt::PythonGILRAII python_gil_raii; + Py_DECREF(fObject); + } + } +}; +} // unnamed namespace + +static PyObject* AsStdAny(PyObject*, PyObject* pyobject) { + // Wrap a reference to pyobject in a std::any owned by Python. + return cpyrt::Instance_FromVoidPtr( + new std::any{std::in_place_type, pyobject}, "std::any", + /*python_owns=*/true); +} + //---------------------------------------------------------------------------- static PyObject* BindValue(PyObject*, PyObject* args, PyObject* kwds) { // Read through the converter for the given type name, at the given address. @@ -949,6 +993,8 @@ static PyMethodDef gcpyrtMethods[] = { (char*)"Create an object of given type, from given address."}, {(char*)"bind_value", (PyCFunction)BindValue, METH_VARARGS | METH_KEYWORDS, (char*)"Read a value of given type, from given address."}, + {(char*)"as_std_any", (PyCFunction)AsStdAny, METH_O, + (char*)"Wrap a reference to a Python object in a std::any."}, {(char*)"move", (PyCFunction)Move, METH_O, (char*)"Cast the C++ object to become movable."}, {(char*)"add_pythonization", (PyCFunction)AddPythonization, METH_VARARGS, diff --git a/bindings/pyroot/cppjit/test/test_lowlevel.py b/bindings/pyroot/cppjit/test/test_lowlevel.py index d741f7f07179f..fc3d70e48002f 100644 --- a/bindings/pyroot/cppjit/test/test_lowlevel.py +++ b/bindings/pyroot/cppjit/test/test_lowlevel.py @@ -1325,3 +1325,60 @@ def test01_cstring_array_from_str(self): s.names = "abc" assert ns.as_chars(s) == "abc" + + +class TestASSTDANY: + def setup_class(cls): + import cppjit + + cppjit.cppdef(r"""\ + #include + #include + #include + namespace AsStdAny { + std::vector gStore; + void store(std::any a) { gStore.push_back(a); } + void clear() { gStore.clear(); } + void clear_in_thread() { std::thread([] { gStore.clear(); }).join(); } + }""") + # clearing in another thread takes the GIL, so it must not be held + cppjit.gbl.AsStdAny.clear_in_thread.__release_gil__ = True + + def _holder(self): + import weakref + + class Holder: + pass + + obj = Holder() + return obj, weakref.ref(obj) + + def test01_keeps_alive(self): + """The std::any and its copies in C++ keep the object alive""" + + import cppjit + import cppjit.ll + + ns = cppjit.gbl.AsStdAny + obj, ref = self._holder() + a = cppjit.ll.as_std_any(obj) + assert type(a) is cppjit.gbl.std.any + ns.store(a) + del obj, a + assert ref() is not None + ns.clear() + assert ref() is None + + def test02_released_in_other_thread(self): + """The last reference can be released by a thread without the GIL""" + + import cppjit + import cppjit.ll + + ns = cppjit.gbl.AsStdAny + obj, ref = self._holder() + ns.store(cppjit.ll.as_std_any(obj)) + del obj + assert ref() is not None + ns.clear_in_thread() + assert ref() is None From b08f89455c65900263d661ceae4572dadcdf59fd Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 8 Oct 2026 20:47:38 +0200 Subject: [PATCH 06/12] [Python] Use cppyy.ll.as_std_any() for the RDataFrame lifelines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NumPy arrays of an RDataFrame made from them, and the TTree of an RDataFrame made from a TTree, were kept alive with a reference in a std::any from the PyObjRefCounterAsStdAny() function of the PyROOT extension module. cppyy.ll.as_std_any() does the same, and also takes the GIL to release the reference, which the destructor of the RDataFrame may do on a thread that does not hold it. 🤖 Done with the help of AI --- .../python/ROOT/_pythonization/_rdataframe.py | 5 +- .../ROOT/_pythonization/_rdf_namespace.py | 6 +- .../pythonizations/src/PyROOTModule.cxx | 82 ------------------- 3 files changed, 6 insertions(+), 87 deletions(-) diff --git a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rdataframe.py b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rdataframe.py index ee50e05e7529f..57a776efd4549 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rdataframe.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rdataframe.py @@ -577,8 +577,9 @@ def _MakeNumpyDataFrame(np_dict): This function takes a dictionary of numpy arrays and creates an RDataFrame using the keys as column names and the numpy arrays as data. """ + import cppyy.ll + import ROOT - from ROOT.libROOTPythonizations import PyObjRefCounterAsStdAny if not isinstance(np_dict, dict): raise RuntimeError("Object not convertible: Python object is not a dictionary.") @@ -594,4 +595,4 @@ def _MakeNumpyDataFrame(np_dict): # original dict, because otherwise the caller of _MakeNumpyDataFrame can # invalidate our cache by mutating the np_dict after the call. - return ROOT.Internal.RDF.MakeRVecDataFrame(PyObjRefCounterAsStdAny(dict(**np_dict)), *args) + return ROOT.Internal.RDF.MakeRVecDataFrame(cppyy.ll.as_std_any(dict(**np_dict)), *args) diff --git a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rdf_namespace.py b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rdf_namespace.py index 150be14261368..18d45c8125b5e 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rdf_namespace.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_rdf_namespace.py @@ -93,8 +93,9 @@ def _rdataframe(local_rdf, distributed_rdf): """ def rdataframe(*args, **kwargs): + import cppyy.ll + import ROOT - from ROOT.libROOTPythonizations import PyObjRefCounterAsStdAny if kwargs.get("executor", None) is not None: rdf = distributed_rdf(*args, **kwargs) @@ -104,8 +105,7 @@ def rdataframe(*args, **kwargs): rnode = ROOT.RDF.AsRNode(rdf) if args and isinstance(args[0], ROOT.TTree): - ROOT.Internal.RDF.SetTTreeLifeline( - rnode, PyObjRefCounterAsStdAny(args[0])) + ROOT.Internal.RDF.SetTTreeLifeline(rnode, cppyy.ll.as_std_any(args[0])) return rdf diff --git a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx index 70a4f4c1fcad5..6a41064f6fceb 100644 --- a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx +++ b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx @@ -24,7 +24,6 @@ #include "RConfigure.h" // Standard -#include #include #include #include @@ -34,67 +33,6 @@ namespace PyROOT { PyObject *gRootModule = nullptr; -/// \brief A PyObject wrapper to track reference counting of external objects -/// -/// This wrapper can be useful in shared ownership scenarios when a C++ object -/// is created on the Python side and there is no easy way to track its ownership -/// on the C++ side. If multiple instances of this class are given the same -/// Python proxy, they will increase/decrease its reference counting following -/// Python rules and ensure proper destruction of the underlying C++ object when -/// no other Python objects are referencing it. -class PyObjRefCounter final { - PyObject *fObject{nullptr}; - - void Reset(PyObject *object) - { - if (fObject) { - Py_DecRef(fObject); - fObject = nullptr; - } - if (object) { - Py_INCREF(object); - fObject = object; - } - } - -public: - PyObjRefCounter(PyObject *object) { Reset(object); } - - ~PyObjRefCounter() { Reset(nullptr); } - - PyObjRefCounter(const PyObjRefCounter &other) { Reset(other.fObject); } - - PyObjRefCounter &operator=(const PyObjRefCounter &other) - { - Reset(other.fObject); - return *this; - } - - PyObjRefCounter(PyObjRefCounter &&other) - { - fObject = other.fObject; - other.fObject = nullptr; - } - - PyObjRefCounter &operator=(PyObjRefCounter &&other) - { - fObject = other.fObject; - other.fObject = nullptr; - return *this; - } -}; - -PyObject *PyObjRefCounterAsStdAny(PyObject * /*self*/, PyObject *args) -{ - PyObject *object = nullptr; - - PyArg_ParseTuple(args, "O:PyObjRefCounterAsStdAny", &object); - - // The std::any is managed by Python - return cppjit::cpyrt::Instance_FromVoidPtr(new std::any{std::in_place_type, object}, "std::any", - /*python_owns=*/true); -} - } // namespace PyROOT namespace { @@ -160,26 +98,6 @@ static PyMethodDef gPyROOTMethods[] = { "Install an input hook to process GUI events"}, {"_CPPInstance__expand__", (PyCFunction)PyROOT::CPPInstanceExpand, METH_VARARGS, "Deserialize a pickled object"}, - {"JupyROOTExecutor", (PyCFunction)JupyROOTExecutor, METH_VARARGS, "Create JupyROOTExecutor"}, - {"JupyROOTDeclarer", (PyCFunction)JupyROOTDeclarer, METH_VARARGS, "Create JupyROOTDeclarer"}, - {"JupyROOTExecutorHandler_Clear", (PyCFunction)JupyROOTExecutorHandler_Clear, METH_NOARGS, - "Clear JupyROOTExecutorHandler"}, - {"JupyROOTExecutorHandler_Ctor", (PyCFunction)JupyROOTExecutorHandler_Ctor, METH_NOARGS, - "Create JupyROOTExecutorHandler"}, - {"JupyROOTExecutorHandler_Poll", (PyCFunction)JupyROOTExecutorHandler_Poll, METH_NOARGS, - "Poll JupyROOTExecutorHandler"}, - {"JupyROOTExecutorHandler_EndCapture", (PyCFunction)JupyROOTExecutorHandler_EndCapture, METH_NOARGS, - "End capture JupyROOTExecutorHandler"}, - {"JupyROOTExecutorHandler_InitCapture", (PyCFunction)JupyROOTExecutorHandler_InitCapture, METH_NOARGS, - "Init capture JupyROOTExecutorHandler"}, - {"JupyROOTExecutorHandler_GetStdout", (PyCFunction)JupyROOTExecutorHandler_GetStdout, METH_NOARGS, - "Get stdout JupyROOTExecutorHandler"}, - {"JupyROOTExecutorHandler_GetStderr", (PyCFunction)JupyROOTExecutorHandler_GetStderr, METH_NOARGS, - "Get stderr JupyROOTExecutorHandler"}, - {"JupyROOTExecutorHandler_Dtor", (PyCFunction)JupyROOTExecutorHandler_Dtor, METH_NOARGS, - "Destruct JupyROOTExecutorHandler"}, - {"PyObjRefCounterAsStdAny", (PyCFunction)PyROOT::PyObjRefCounterAsStdAny, METH_VARARGS, - "Wrap a reference count to any Python object in a std::any for resource management in C++"}, {NULL, NULL, 0, NULL}}; struct module_state { From 737c5b658e50c66203649b533beeb916d46bfe94 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 8 Oct 2026 21:11:58 +0200 Subject: [PATCH 07/12] [core] Add ROOT::Internal::GetBufferAddress() for the Python bindings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TBuffer::Buffer() returns a char*, which the Python bindings see as a string, with the address of the buffer gone. Pickling ROOT objects from Python needs that address to read the streamed bytes. 🤖 Done with the help of AI --- core/base/inc/TBuffer.h | 6 ++++++ core/base/src/TBuffer.cxx | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/core/base/inc/TBuffer.h b/core/base/inc/TBuffer.h index eb25fc6f84649..f9d2b0498fc82 100644 --- a/core/base/inc/TBuffer.h +++ b/core/base/inc/TBuffer.h @@ -408,4 +408,10 @@ inline Int_t TBuffer::WriteObject(const T *objptr, Bool_t cacheReuse) return WriteObjectAny(objptr, cl, cacheReuse); } +namespace ROOT::Internal { +// For the Python bindings, which see the char* that TBuffer::Buffer() returns +// as a string. +Longptr_t GetBufferAddress(const TBuffer &buffer); +} // namespace ROOT::Internal + #endif // ROOT_TBuffer diff --git a/core/base/src/TBuffer.cxx b/core/base/src/TBuffer.cxx index 20cba8b524a37..ec0ed764a3a04 100644 --- a/core/base/src/TBuffer.cxx +++ b/core/base/src/TBuffer.cxx @@ -426,3 +426,10 @@ Bool_t TBuffer::ByteSwapBuffer(Long64_t n, EDataType type) return true; } + +//////////////////////////////////////////////////////////////////////////////// + +Longptr_t ROOT::Internal::GetBufferAddress(const TBuffer &buffer) +{ + return reinterpret_cast(buffer.Buffer()); +} From 0d3c90a89740d264854364b7308dcf82c8fde316 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 8 Oct 2026 21:13:03 +0200 Subject: [PATCH 08/12] [Python] Move the pickling of C++ objects to Python MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pickling of C++ objects, by streaming them with ROOT I/O, was implemented in CPPInstancePyz.cxx of the PyROOT extension module, which installed a C function as the __reduce__ method of all cppjit instances. It is now Python: _reduce() streams the object with TBufferFile, reading the streamed bytes with ROOT::Internal::GetBufferAddress(), and _expand() reads it back. For that, the __reduce__ hook of cppjit holds a Python callable instead of a C function, set with libcppjit._set_reduce_method(). Pickles of the previous implementation can't be read back: they name the expand function of the removed extension module. 🤖 Done with the help of AI --- bindings/pyroot/cppjit/src/cpyrt/API.cxx | 6 +- bindings/pyroot/cppjit/src/cpyrt/API.h | 5 +- .../pyroot/cppjit/src/cpyrt/CPPInstance.cxx | 10 +- .../pyroot/cppjit/src/cpyrt/CPPInstance.h | 8 +- .../pyroot/cppjit/src/cpyrt/cpyrtModule.cxx | 18 +++ bindings/pyroot/cppjit/test/test_lowlevel.py | 22 ++++ bindings/pyroot/pythonizations/CMakeLists.txt | 1 - .../ROOT/_pythonization/_cppinstance.py | 57 ++++++++- .../pythonizations/src/CPPInstancePyz.cxx | 121 ------------------ .../pythonizations/src/PyROOTModule.cxx | 7 - .../pythonizations/src/PyROOTPythonize.h | 25 ---- 11 files changed, 112 insertions(+), 168 deletions(-) delete mode 100644 bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx delete mode 100644 bindings/pyroot/pythonizations/src/PyROOTPythonize.h diff --git a/bindings/pyroot/cppjit/src/cpyrt/API.cxx b/bindings/pyroot/cppjit/src/cpyrt/API.cxx index b9724f87af8d0..20208aef4761b 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/API.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/API.cxx @@ -302,8 +302,12 @@ bool cpyrt::Overload_CheckExact(PyObject* pyobject) { } //----------------------------------------------------------------------------- -void cpyrt::Instance_SetReduceMethod(PyCFunction reduceMethod) { +void cpyrt::Instance_SetReduceMethod(PyObject* reduceMethod) { + PythonGILRAII python_gil_raii; + Py_XINCREF(reduceMethod); + PyObject* previous = CPPInstance::ReduceMethod(); CPPInstance::ReduceMethod() = reduceMethod; + Py_XDECREF(previous); } //- access to the python interpreter ---------------------------------------- diff --git a/bindings/pyroot/cppjit/src/cpyrt/API.h b/bindings/pyroot/cppjit/src/cpyrt/API.h index 627132a2e265b..be1a2285b53f7 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/API.h +++ b/bindings/pyroot/cppjit/src/cpyrt/API.h @@ -229,8 +229,9 @@ CPYRT_EXTERN bool Overload_CheckExact(PyObject* pyobject); // Sets the __reduce__ method for the CPPInstance class, which is by default not // implemented by cppjit but might make sense to implement by frameworks that -// support IO of arbitrary C++ objects, like ROOT. -CPYRT_EXTERN void Instance_SetReduceMethod(PyCFunction reduceMethod); +// support IO of arbitrary C++ objects, like ROOT. reduceMethod is a Python +// callable taking the instance, or nullptr to unset it. +CPYRT_EXTERN void Instance_SetReduceMethod(PyObject* reduceMethod); //- access to the python interpreter ---------------------------------------- diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.cxx index 13ccf3a7b02d5..5171eb59bfe0d 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.cxx @@ -412,18 +412,18 @@ static PySequenceMethods op_as_sequence = { 0, // sq_inplace_repeat }; -PyCFunction& CPPInstance::ReduceMethod() { - static PyCFunction reducer = nullptr; +PyObject*& CPPInstance::ReduceMethod() { + static PyObject* reducer = nullptr; return reducer; } -PyObject* op_reduce(PyObject* self, PyObject* args) { - auto& reducer = CPPInstance::ReduceMethod(); +PyObject* op_reduce(PyObject* self, PyObject* /* args */) { + PyObject* reducer = CPPInstance::ReduceMethod(); if (!reducer) { PyErr_SetString(PyExc_NotImplementedError, ""); return nullptr; } - return reducer(self, args); + return PyObject_CallFunctionObjArgs(reducer, self, nullptr); } //---------------------------------------------------------------------------- diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.h b/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.h index c94ecfa16c220..bf6d6cf5c21e0 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.h @@ -85,10 +85,10 @@ class CPPInstance { void CastToArray(Py_ssize_t sz); Py_ssize_t ArrayLength(); - // implementation of the __reduce__ method: doesn't wrap any function by - // default but can be re-assigned by libraries that add C++ object - // serialization support, like ROOT - static PyCFunction& ReduceMethod(); + // implementation of the __reduce__ method: a Python callable taking the + // instance, unset by default but can be set by libraries that add C++ + // object serialization support, like ROOT + static PyObject*& ReduceMethod(); private: void CreateExtension(); diff --git a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx index 701809b496280..d0f28e68567db 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx @@ -667,6 +667,22 @@ static PyObject* BindObject(PyObject*, PyObject* args, PyObject* kwds) { return BindCppObjectNoCast(addr, cast_type); } +//---------------------------------------------------------------------------- +static PyObject* SetReduceMethod(PyObject*, PyObject* reducer) { + // Set the __reduce__ method of all instances; None unsets it. + if (reducer == Py_None) + reducer = nullptr; + else if (!PyCallable_Check(reducer)) { + PyErr_SetString(PyExc_TypeError, "reduce method must be callable or None"); + return nullptr; + } + Py_XINCREF(reducer); + PyObject* previous = CPPInstance::ReduceMethod(); + CPPInstance::ReduceMethod() = reducer; + Py_XDECREF(previous); + Py_RETURN_NONE; +} + //---------------------------------------------------------------------------- namespace { // A reference to a Python object, held by C++. Copies hold references of @@ -1003,6 +1019,8 @@ static PyMethodDef gcpyrtMethods[] = { METH_VARARGS, (char*)"Remove a pythonizor."}, {(char*)"_pin_type", (PyCFunction)PinType, METH_O, (char*)"Install a type pinning."}, + {(char*)"_set_reduce_method", (PyCFunction)SetReduceMethod, METH_O, + (char*)"Set the __reduce__ method of all instances."}, {(char*)"_add_type_reducer", (PyCFunction)AddTypeReducer, METH_VARARGS, (char*)"Add a type reducer."}, {(char*)"SetHeuristicMemoryPolicy", (PyCFunction)SetHeuristicMemoryPolicy, diff --git a/bindings/pyroot/cppjit/test/test_lowlevel.py b/bindings/pyroot/cppjit/test/test_lowlevel.py index fc3d70e48002f..1971c9a831b59 100644 --- a/bindings/pyroot/cppjit/test/test_lowlevel.py +++ b/bindings/pyroot/cppjit/test/test_lowlevel.py @@ -1382,3 +1382,25 @@ def test02_released_in_other_thread(self): assert ref() is not None ns.clear_in_thread() assert ref() is None + + +class TestREDUCEMETHOD: + def test01_set_and_unset(self): + """The __reduce__ method of all instances can be set and unset""" + + import pickle + + import cppjit + + cppjit.cppdef("namespace ReduceMethod { struct S { int x = 3; }; }") + + def reduce_s(obj): + return (int, (obj.x,)) + + cppjit._backend._set_reduce_method(reduce_s) + try: + assert pickle.loads(pickle.dumps(cppjit.gbl.ReduceMethod.S())) == 3 + finally: + cppjit._backend._set_reduce_method(None) + raises(NotImplementedError, pickle.dumps, cppjit.gbl.ReduceMethod.S()) + raises(TypeError, cppjit._backend._set_reduce_method, 42) diff --git a/bindings/pyroot/pythonizations/CMakeLists.txt b/bindings/pyroot/pythonizations/CMakeLists.txt index 70a847b645c07..a0a099da019c6 100644 --- a/bindings/pyroot/pythonizations/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/CMakeLists.txt @@ -10,7 +10,6 @@ set(cpp_sources src/PyROOTModule.cxx - src/CPPInstancePyz.cxx ) set(libname ROOTPythonizations) diff --git a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_cppinstance.py b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_cppinstance.py index a16dedc295b79..b463993e8e341 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_cppinstance.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_cppinstance.py @@ -9,10 +9,63 @@ ################################################################################ +import ctypes + + +def _expand(data, class_name): + """ + Read back an object pickled by _reduce(): data holds the object as + streamed by ROOT I/O, class_name is the name of its class. + """ + import ROOT + + if class_name == "TBufferFile": + # TBuffer and its derived classes can't stream themselves, but can be + # created from their contents + buf = ROOT.TBufferFile(ROOT.TBuffer.kWrite) + buf.WriteFastArray(data, len(data)) + return buf + + # The buffer is read from in place: keep it alive while it is read + raw = bytearray(data) + buf = ROOT.TBufferFile(ROOT.TBuffer.kRead, len(raw), raw, False) + address = ROOT.addressof(buf.ReadObjectAny(ROOT.nullptr)) + obj = ROOT.bind_object(address, class_name) + ROOT.SetOwnership(obj, True) + return obj + + +def _reduce(obj): + """ + The __reduce__ method of all C++ instances: stream the object with ROOT + I/O, to be read back by _expand(). + """ + import ROOT + + class_name = type(obj).__cpp_name__ + if class_name == "TBufferFile": + # TBuffer and its derived classes can't stream themselves: pickle + # their contents + buf = obj + else: + if class_name.startswith("__cppjit_internal::Dispatcher"): + raise OSError( + "generic streaming of Python objects whose class derives from a C++ class is not supported. " + "Please refer to the Python pickle documentation for instructions on how to define " + "a custom __reduce__ method for the derived Python class" + ) + buf = ROOT.TBufferFile(ROOT.TBuffer.kWrite) + if buf.WriteObjectAny(obj, ROOT.TClass.GetClass(class_name)) != 1: + raise OSError("could not stream object of type {}".format(class_name)) + + data = ctypes.string_at(ROOT.Internal.GetBufferAddress(buf), buf.Length()) + return _expand, (data, class_name) + + def pythonize_cppinstance(): - from ROOT.libROOTPythonizations import AddCPPInstancePickling + import cppyy - AddCPPInstancePickling() + cppyy._backend._set_reduce_method(_reduce) # Instant pythonization (executed at `import ROOT` time), no need of a diff --git a/bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx b/bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx deleted file mode 100644 index a1438543b08c6..0000000000000 --- a/bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx +++ /dev/null @@ -1,121 +0,0 @@ -// Author: Massimiliano Galli CERN 07/2019 -// Original PyROOT code by Wim Lavrijsen, LBL - -/************************************************************************* - * Copyright (C) 1995-2018, Rene Brun and Fons Rademakers. * - * All rights reserved. * - * * - * For the licensing terms see $ROOTSYS/LICENSE. * - * For the list of contributors see $ROOTSYS/README/CREDITS. * - *************************************************************************/ - -#include - -// Bindings -#include "cpyrt/API.h" - -#include "PyROOTPythonize.h" -#include "TBufferFile.h" - -namespace PyROOT { -extern PyObject *gRootModule; -} - -//////////////////////////////////////////////////////////////////////////// -/// \brief Deserialize pickled objects -/// \param[in] self Always null, since this is a module function. -/// \param[in] args Pointer to a Python tuple object containing the arguments -/// received from Python. -/// -/// Helper function that deserializes pickled objects. It needs to be -/// included in the extension module API because otherwise it is not -/// callable from Python. This is important because it will be Python -/// itself calling it when trying to expand a serialized object. -PyObject *PyROOT::CPPInstanceExpand(PyObject * /*self*/, PyObject *args) -{ - PyObject *pybuf = 0, *pyname = 0; - if (!PyArg_ParseTuple(args, "O!O!:__expand__", &PyBytes_Type, &pybuf, &PyBytes_Type, &pyname)) - return 0; - const char *clname = PyBytes_AsString(pyname); - // TBuffer and its derived classes can't write themselves, but can be created - // directly from the buffer, so handle them in a special case - void *newObj = 0; - if (strcmp(clname, "TBufferFile") == 0) { - TBufferFile *buf = new TBufferFile(TBuffer::kWrite); - buf->WriteFastArray(PyBytes_AsString(pybuf), PyBytes_Size(pybuf)); - newObj = buf; - } else { - // use the PyString macro's to by-pass error checking; do not adopt the buffer, - // as the local TBufferFile can go out of scope (there is no copying) - TBufferFile buf(TBuffer::kRead, PyBytes_Size(pybuf), PyBytes_AsString(pybuf), kFALSE); - newObj = buf.ReadObjectAny(0); - } - PyObject *result = cppjit::cpyrt::Instance_FromVoidPtr(newObj, clname, /*python_owns=*/true); - return result; -} - -/// PyROOT object proxy pickle support -/// Turn the object proxy instance into a character stream and return for -/// pickle, together with the callable object that can restore the stream -/// into the object proxy instance. -PyObject *op_reduce(PyObject *self, PyObject * /*args*/) -{ - // keep a borrowed reference around to the callable function for expanding; - // because it is borrowed, it means that there can be no pickling during the - // shutdown of the libPyROOT module - static PyObject *s_expand = PyDict_GetItemString(PyModule_GetDict(PyROOT::gRootModule), "_CPPInstance__expand__"); - - // TBuffer and its derived classes can't write themselves, but can be created - // directly from the buffer, so handle them in a special case - TBufferFile *buff = nullptr; - std::string className = cppjit::cpyrt::Instance_GetScopedFinalName(self); - if (className == "TBufferFile") { - buff = (TBufferFile *)cppjit::cpyrt::Instance_AsVoidPtr(self); - } else { - if (className.find("__cppjit_internal::Dispatcher") == 0) { - PyErr_Format(PyExc_IOError, - "generic streaming of Python objects whose class derives from a C++ class is not supported. " - "Please refer to the Python pickle documentation for instructions on how to define " - "a custom __reduce__ method for the derived Python class"); - return 0; - } - // no cast is needed, but WriteObject taking a TClass argument is protected, - // so use WriteObjectAny() - static TBufferFile s_buff(TBuffer::kWrite); - s_buff.Reset(); - // to delete - if (s_buff.WriteObjectAny(cppjit::cpyrt::Instance_AsVoidPtr(self), TClass::GetClass(className.c_str())) != 1) { - PyErr_Format(PyExc_IOError, "could not stream object of type %s", - className.c_str()); - return 0; - } - buff = &s_buff; - } - // use a string for the serialized result, as a python buffer will not copy - // the buffer contents; use a string for the class name, used when casting - // on reading back in (see CPPInstanceExpand defined above) - PyObject *res2 = PyTuple_New(2); - PyTuple_SetItem(res2, 0, PyBytes_FromStringAndSize(buff->Buffer(), buff->Length())); - PyTuple_SetItem(res2, 1, PyBytes_FromString(className.c_str())); - - PyObject *result = PyTuple_New(2); - Py_INCREF(s_expand); - PyTuple_SetItem(result, 0, s_expand); - PyTuple_SetItem(result, 1, res2); - - return result; -} - -//////////////////////////////////////////////////////////////////////////// -/// \brief Set __reduce__ attribute for CPPInstance objects -/// \param[in] self Always null, since this is a module function. -/// \param[in] args Pointer to a Python tuple object containing the arguments -/// received from Python. -/// -/// The C++ function op_reduce defined above is wrapped in a Python method -/// so that it can be injected in CPPInstance -PyObject *PyROOT::AddCPPInstancePickling(PyObject * /*self*/, PyObject * /*args*/) -{ - cppjit::cpyrt::Instance_SetReduceMethod((PyCFunction)op_reduce); - Py_RETURN_NONE; -} diff --git a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx index 6a41064f6fceb..3c7c13b446a42 100644 --- a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx +++ b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx @@ -9,9 +9,6 @@ * For the list of contributors see $ROOTSYS/README/CREDITS. * *************************************************************************/ -// Bindings -#include "PyROOTPythonize.h" - // Cppyy #include "cpyrt/API.h" @@ -90,14 +87,10 @@ PyObject *InstallGUIEventInputHook(PyObject * /*self*/, PyObject * /*args*/) // Methods offered by the interface static PyMethodDef gPyROOTMethods[] = { - {"AddCPPInstancePickling", (PyCFunction)PyROOT::AddCPPInstancePickling, METH_NOARGS, - "Add a custom pickling mechanism for Cppyy Python proxy objects"}, {"InstallWarningHandler", (PyCFunction)InstallWarningHandler, METH_NOARGS, "Turn ROOT warnings into Python warnings"}, {"InstallGUIEventInputHook", (PyCFunction)InstallGUIEventInputHook, METH_NOARGS, "Install an input hook to process GUI events"}, - {"_CPPInstance__expand__", (PyCFunction)PyROOT::CPPInstanceExpand, METH_VARARGS, - "Deserialize a pickled object"}, {NULL, NULL, 0, NULL}}; struct module_state { diff --git a/bindings/pyroot/pythonizations/src/PyROOTPythonize.h b/bindings/pyroot/pythonizations/src/PyROOTPythonize.h deleted file mode 100644 index 0c948bbd568f9..0000000000000 --- a/bindings/pyroot/pythonizations/src/PyROOTPythonize.h +++ /dev/null @@ -1,25 +0,0 @@ -// Author: Enric Tejedor CERN 06/2018 -// Original PyROOT code by Wim Lavrijsen, LBL - -/************************************************************************* - * Copyright (C) 1995-2018, Rene Brun and Fons Rademakers. * - * All rights reserved. * - * * - * For the licensing terms see $ROOTSYS/LICENSE. * - * For the list of contributors see $ROOTSYS/README/CREDITS. * - *************************************************************************/ - -#ifndef PYROOT_PYTHONIZE_H -#define PYROOT_PYTHONIZE_H - -#include - -namespace PyROOT { - -PyObject *AddCPPInstancePickling(PyObject *self, PyObject *args); - -PyObject *CPPInstanceExpand(PyObject *self, PyObject *args); - -} // namespace PyROOT - -#endif // !PYROOT_PYTHONIZE_H From de8f51442fcfc2726910fd07cd6f30ad22c3cd00 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 13 Oct 2026 18:25:43 +0000 Subject: [PATCH 09/12] [core] Add ROOT::Internal::ProcessGUIEvents() for the Python bindings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PyROOT's PyOS_InputHook processes GUI events while the Python prompt waits for input, updating a web canvas first so that its pending drawing is part of them. The hook lives in the Python bindings' CPython extension module, which is going away: the ROOT part of it is now a compiled helper in Core, which needs no Python, so the Python bindings can install it from Python code. 🤖 Done with the help of AI --- core/base/inc/TApplication.h | 6 ++++++ core/base/src/TApplication.cxx | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/core/base/inc/TApplication.h b/core/base/inc/TApplication.h index f3484f5f4a2b2..7a2043d1174e2 100644 --- a/core/base/inc/TApplication.h +++ b/core/base/inc/TApplication.h @@ -169,4 +169,10 @@ class TApplication : public TObject, public TQObject { R__EXTERN TApplication *gApplication; +namespace ROOT::Internal { +/// For the Python bindings: process the pending GUI events, as +/// TApplication::Run() does between the commands of an interactive session. +void ProcessGUIEvents(); +} // namespace ROOT::Internal + #endif diff --git a/core/base/src/TApplication.cxx b/core/base/src/TApplication.cxx index 89aea7efa73ea..9508fdd7f02da 100644 --- a/core/base/src/TApplication.cxx +++ b/core/base/src/TApplication.cxx @@ -2190,3 +2190,14 @@ TList *TApplication::GetApplications() { return fgApplications; } + +//////////////////////////////////////////////////////////////////////////////// +/// Process the pending GUI events, updating a web canvas first so that its +/// pending drawing is part of them. + +void ROOT::Internal::ProcessGUIEvents() +{ + if (gPad && gPad->IsWeb()) + gPad->UpdateAsync(); + gSystem->ProcessEvents(); +} From 03f3755dbbaaee9d04f9670aefe0a5f2f9d5d946 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 13 Oct 2026 18:33:47 +0000 Subject: [PATCH 10/12] [cppjit] Add _backend.warning_handler() for warnings reported by C++ [upstream] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C++ libraries commonly report warnings to a callback. ROOT's Python bindings turn them into Python warnings, which only a compiled handler can do: it has to take the GIL, decline the warning while a Python exception is pending (such as a previous warning that the filters turned into an error during the same call into C++) and leave the exception a raised warning sets pending until C++ returns to Python. A ctypes callback prints and clears that exception instead, and a cppjit callback trampoline makes cling parse Python.h, which is too slow for every import. warning_handler() hands out such a handler as a ctypes function pointer, to be passed to the library's registration function. It takes an optional category, RuntimeWarning by default, and the module name the warnings filters match against, so that warnings from one library can be filtered as a group. 🤖 Done with the help of AI --- .../pyroot/cppjit/src/cpyrt/cpyrtModule.cxx | 78 +++++++++++++++++++ bindings/pyroot/cppjit/test/test_lowlevel.py | 76 ++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx index d0f28e68567db..2b5c648888bb8 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx @@ -726,6 +726,80 @@ static PyObject* AsStdAny(PyObject*, PyObject* pyobject) { /*python_owns=*/true); } +//---------------------------------------------------------------------------- +namespace { + +PyObject* gWarningCategory = nullptr; // nullptr: RuntimeWarning +std::string gWarningModule; + +bool WarningHandler(const char* location, const char* msg) { + // Issue a Python warning for a warning that C++ reports, see + // warning_handler() below. Called from C++, maybe without the GIL. + cpyrt::PythonGILRAII gil; + + // Python code can't run while an exception is pending, e.g. a previous + // warning that the filters turned into an error during the same call into + // C++: decline, so that C++ reports the warning its own way. + if (PyErr_Occurred()) + return false; + + // Prints or, per the filters, raises; a raised exception stays pending + // until the call into C++ returns to Python. + PyErr_WarnExplicit(gWarningCategory, msg, location, 0, + gWarningModule.empty() ? nullptr : gWarningModule.c_str(), + nullptr); + return true; +} + +} // unnamed namespace + +static PyObject* GetWarningHandler(PyObject*, PyObject* args, PyObject* kwds) { + // Configure the compiled warning handler and hand it out as a ctypes + // function pointer, for C++ libraries that report warnings to a callback. + static const char* kwlist[] = {(char*)"category", (char*)"module", nullptr}; + + PyObject* category = nullptr; + const char* module = nullptr; + if (!PyArg_ParseTupleAndKeywords(args, kwds, "|Oz:warning_handler", + (char**)kwlist, &category, &module)) + return nullptr; + + if (category == Py_None) + category = nullptr; + if (category && PyObject_IsSubclass(category, PyExc_Warning) != 1) { + if (!PyErr_Occurred()) + PyErr_SetString(PyExc_TypeError, + "category must be a subclass of Warning"); + return nullptr; + } + + Py_XINCREF(category); + Py_XSETREF(gWarningCategory, category); + gWarningModule = module ? module : ""; + + // ctypes.CFUNCTYPE(c_bool, c_char_p, c_char_p)(address) + PyObject* ctmod = PyImport_ImportModule("ctypes"); + if (!ctmod) + return nullptr; + PyObject* c_bool = PyObject_GetAttrString(ctmod, "c_bool"); + PyObject* c_char_p = PyObject_GetAttrString(ctmod, "c_char_p"); + PyObject* fntype = nullptr; + if (c_bool && c_char_p) + fntype = PyObject_CallMethod(ctmod, "CFUNCTYPE", "OOO", c_bool, c_char_p, + c_char_p); + Py_XDECREF(c_char_p); + Py_XDECREF(c_bool); + Py_DECREF(ctmod); + if (!fntype) + return nullptr; + + PyObject* address = PyLong_FromVoidPtr((void*)&WarningHandler); + PyObject* result = PyObject_CallOneArg(fntype, address); + Py_DECREF(address); + Py_DECREF(fntype); + return result; +} + //---------------------------------------------------------------------------- static PyObject* BindValue(PyObject*, PyObject* args, PyObject* kwds) { // Read through the converter for the given type name, at the given address. @@ -1011,6 +1085,10 @@ static PyMethodDef gcpyrtMethods[] = { (char*)"Read a value of given type, from given address."}, {(char*)"as_std_any", (PyCFunction)AsStdAny, METH_O, (char*)"Wrap a reference to a Python object in a std::any."}, + {(char*)"warning_handler", (PyCFunction)GetWarningHandler, + METH_VARARGS | METH_KEYWORDS, + (char*)"Get a C function pointer that issues Python warnings for " + "warnings reported by C++."}, {(char*)"move", (PyCFunction)Move, METH_O, (char*)"Cast the C++ object to become movable."}, {(char*)"add_pythonization", (PyCFunction)AddPythonization, METH_VARARGS, diff --git a/bindings/pyroot/cppjit/test/test_lowlevel.py b/bindings/pyroot/cppjit/test/test_lowlevel.py index 1971c9a831b59..e54044b1c99a7 100644 --- a/bindings/pyroot/cppjit/test/test_lowlevel.py +++ b/bindings/pyroot/cppjit/test/test_lowlevel.py @@ -1404,3 +1404,79 @@ def reduce_s(obj): cppjit._backend._set_reduce_method(None) raises(NotImplementedError, pickle.dumps, cppjit.gbl.ReduceMethod.S()) raises(TypeError, cppjit._backend._set_reduce_method, 42) + + +class TestWARNINGHANDLER: + def setup_class(cls): + import cppjit + + cppjit.cppdef(r"""\ + namespace WarningHandler { + using handler_t = bool (*)(const char *location, const char *msg); + handler_t gHandler = nullptr; + int gHandled = 0; + void set(handler_t h) { gHandler = h; } + void warn_twice() { + gHandled = gHandler("first", "message 1"); + gHandled += gHandler("second", "message 2"); + } + }""") + + def test01_warnings(self): + """C++ warnings become Python warnings with the location as file name""" + + import warnings + + import cppjit + + ns = cppjit.gbl.WarningHandler + ns.set(cppjit._backend.warning_handler()) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + ns.warn_twice() + assert ns.gHandled == 2 + assert [(w.category, str(w.message), w.filename, w.lineno) for w in caught] == [ + (RuntimeWarning, "message 1", "first", 0), + (RuntimeWarning, "message 2", "second", 0), + ] + + def test02_category_and_module(self): + """The category and the module the filters see can be configured""" + + import warnings + + import cppjit + + ns = cppjit.gbl.WarningHandler + ns.set( + cppjit._backend.warning_handler( + category=UserWarning, module="warning_handler_test" + ) + ) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("ignore") + warnings.filterwarnings("always", module="warning_handler_test") + ns.warn_twice() + assert [w.category for w in caught] == [UserWarning, UserWarning] + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("ignore") + warnings.filterwarnings("always", module="other_module") + ns.warn_twice() + assert not caught + raises(TypeError, cppjit._backend.warning_handler, category=int) + + def test03_raised_and_declined(self): + """A warning turned into an error is raised once C++ returns; until + then, further warnings are declined""" + + import warnings + + import cppjit + + ns = cppjit.gbl.WarningHandler + ns.set(cppjit._backend.warning_handler()) + with warnings.catch_warnings(): + warnings.simplefilter("error") + with raises(RuntimeWarning, match="message 1"): + ns.warn_twice() + assert ns.gHandled == 1 From 4426a1976f7e477d10652d6d75c66afd929ab765 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 13 Oct 2026 18:35:21 +0000 Subject: [PATCH 11/12] [cppjit] Add _backend.set_input_hook() to run C++ while the prompt waits [upstream] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A C++ GUI toolkit bound with cppjit has to process its events while the interactive Python prompt waits for input, which is what PyOS_InputHook is for. The hook has to be compiled: it must take the GIL, so that event processing never runs alongside other Python threads, and it must run no Python code, since a ctypes callback swallows the KeyboardInterrupt of a Ctrl-C arriving while it runs and the Ctrl-C is lost. set_input_hook() installs a hook that calls the given compiled C++ function with the GIL and then the hook that was installed before; None restores the previous hook. 🤖 Done with the help of AI --- .../pyroot/cppjit/src/cpyrt/cpyrtModule.cxx | 55 ++++++++++++++++++ bindings/pyroot/cppjit/test/test_lowlevel.py | 58 +++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx index 2b5c648888bb8..82b3dce77c240 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx @@ -800,6 +800,58 @@ static PyObject* GetWarningHandler(PyObject*, PyObject* args, PyObject* kwds) { return result; } +//---------------------------------------------------------------------------- +namespace { + +void (*gInputHookFunc)() = nullptr; +int (*gPreviousInputHook)() = nullptr; + +int InputHook() { + // The PyOS_InputHook set_input_hook() installs: calls the C++ function + // with the GIL, then the previous hook. Runs no Python code, so a Ctrl-C + // arriving meanwhile stays pending for the prompt. + { + cpyrt::PythonGILRAII gil; + gInputHookFunc(); + } + return gPreviousInputHook ? gPreviousInputHook() : 0; +} + +} // unnamed namespace + +static PyObject* SetInputHook(PyObject*, PyObject* pyfunc) { + // Install the PyOS_InputHook that calls the compiled C++ function pyfunc, + // a void() overload, while the interactive prompt waits for input; None + // restores the previous hook. + if (pyfunc == Py_None) { + if (PyOS_InputHook == &InputHook) { + PyOS_InputHook = gPreviousInputHook; + gPreviousInputHook = nullptr; + } + gInputHookFunc = nullptr; + Py_RETURN_NONE; + } + + interop::TCppFuncAddr_t addr = 0; + if (CPPOverload_CheckExact(pyfunc)) { + const auto& methods = ((CPPOverload*)pyfunc)->fMethodInfo->fMethods; + if (methods.size() == 1 && methods[0]->GetMaxArgs() == 0) + addr = methods[0]->GetFunctionAddress(); + } + if (!addr) { + PyErr_SetString(PyExc_TypeError, + "a C++ function taking no arguments expected"); + return nullptr; + } + + gInputHookFunc = (void (*)())addr; + if (PyOS_InputHook != &InputHook) { + gPreviousInputHook = PyOS_InputHook; + PyOS_InputHook = &InputHook; + } + Py_RETURN_NONE; +} + //---------------------------------------------------------------------------- static PyObject* BindValue(PyObject*, PyObject* args, PyObject* kwds) { // Read through the converter for the given type name, at the given address. @@ -1089,6 +1141,9 @@ static PyMethodDef gcpyrtMethods[] = { METH_VARARGS | METH_KEYWORDS, (char*)"Get a C function pointer that issues Python warnings for " "warnings reported by C++."}, + {(char*)"set_input_hook", (PyCFunction)SetInputHook, METH_O, + (char*)"Call a C++ function while the interactive prompt waits for " + "input."}, {(char*)"move", (PyCFunction)Move, METH_O, (char*)"Cast the C++ object to become movable."}, {(char*)"add_pythonization", (PyCFunction)AddPythonization, METH_VARARGS, diff --git a/bindings/pyroot/cppjit/test/test_lowlevel.py b/bindings/pyroot/cppjit/test/test_lowlevel.py index e54044b1c99a7..7e8a1a7e9bed8 100644 --- a/bindings/pyroot/cppjit/test/test_lowlevel.py +++ b/bindings/pyroot/cppjit/test/test_lowlevel.py @@ -1480,3 +1480,61 @@ def test03_raised_and_declined(self): with raises(RuntimeWarning, match="message 1"): ns.warn_twice() assert ns.gHandled == 1 + + +class TestINPUTHOOK: + def setup_class(cls): + import cppjit + + cppjit.cppdef(r"""\ + namespace InputHook { + int gCalls = 0; + void hook() { ++gCalls; } + void with_arg(int) {} + }""") + + def test01_install_and_chain(self): + """The hook calls the C++ function, then the previous hook""" + + import ctypes + + import cppjit + + ns = cppjit.gbl.InputHook + hook_type = ctypes.CFUNCTYPE(ctypes.c_int) + hook_ptr = ctypes.c_void_p.in_dll(ctypes.pythonapi, "PyOS_InputHook") + original = hook_ptr.value + + previous_calls = [] + previous_hook = hook_type(lambda: previous_calls.append(1) or 42) + previous_address = ctypes.cast(previous_hook, ctypes.c_void_p).value + hook_ptr.value = previous_address + try: + cppjit._backend.set_input_hook(ns.hook) + installed = hook_ptr.value + assert installed != previous_address + assert hook_type(installed)() == 42 + assert ns.gCalls == 1 + assert previous_calls == [1] + + # installing again replaces the function, not the chain + cppjit._backend.set_input_hook(ns.hook) + assert hook_ptr.value == installed + assert hook_type(installed)() == 42 + assert ns.gCalls == 2 + assert previous_calls == [1, 1] + + cppjit._backend.set_input_hook(None) + assert hook_ptr.value == previous_address + finally: + hook_ptr.value = original + + def test02_rejects_other_arguments(self): + """Only a C++ function taking no arguments can be the hook""" + + import cppjit + + ns = cppjit.gbl.InputHook + raises(TypeError, cppjit._backend.set_input_hook, ns.with_arg) + raises(TypeError, cppjit._backend.set_input_hook, lambda: None) + raises(TypeError, cppjit._backend.set_input_hook, 42) From c3be4a1895079f4763735931b84732aaaa12bb1c Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 13 Oct 2026 18:35:47 +0000 Subject: [PATCH 12/12] [Python] Remove the libROOTPythonizations CPython extension module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its last two functions installed the error handler that turns ROOT warnings into Python warnings and the PyOS_InputHook that processes GUI events while the Python prompt waits for input. Both have to be compiled, and cppjit now provides them: _backend.warning_handler() hands out the handler for ROOT::Internal::SetWarningHandler(), and _backend.set_input_hook() installs the hook that calls the new ROOT::Internal::ProcessGUIEvents(). ROOT's Python package now consists of Python code only, on top of the cppjit extension, and no ROOT library outside cppjit includes Python.h. 🤖 Done with the help of AI --- bindings/pyroot/cppjit/src/CMakeLists.txt | 2 +- bindings/pyroot/pythonizations/CMakeLists.txt | 68 +-------- .../pythonizations/python/ROOT/__init__.py | 8 +- .../python/ROOT/_application.py | 9 +- .../pythonizations/src/PyROOTModule.cxx | 133 ------------------ .../pythonizations/test/import_load_libs.py | 1 - .../test/regression_pickle_no_cppyy.py | 10 +- bindings/pyroot/pythonizations/test/th1.py | 2 +- 8 files changed, 16 insertions(+), 217 deletions(-) delete mode 100644 bindings/pyroot/pythonizations/src/PyROOTModule.cxx diff --git a/bindings/pyroot/cppjit/src/CMakeLists.txt b/bindings/pyroot/cppjit/src/CMakeLists.txt index b63ff156f05c1..ba8f358bd33cf 100644 --- a/bindings/pyroot/cppjit/src/CMakeLists.txt +++ b/bindings/pyroot/cppjit/src/CMakeLists.txt @@ -32,7 +32,7 @@ target_compile_definitions(cppjit_backend PRIVATE # through the dispatch mechanism. Disable them in this translation unit. NDEBUG # libcpyrt owns the dispatch table (cppinterop_dispatch.cxx); consumers in - # other DLLs (ROOTPythonizations, tpython) import it. On Windows the data + # other DLLs (tpython) import it. On Windows the data # symbols need explicit dllexport - WINDOWS_EXPORT_ALL_SYMBOLS covers # functions only. CPPINTEROP_DISPATCH_EXPORTS diff --git a/bindings/pyroot/pythonizations/CMakeLists.txt b/bindings/pyroot/pythonizations/CMakeLists.txt index a0a099da019c6..5813e4b33365a 100644 --- a/bindings/pyroot/pythonizations/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/CMakeLists.txt @@ -5,77 +5,13 @@ # For the list of contributors see $ROOTSYS/README/CREDITS. ################################################################ -# CMakeLists.txt file for building ROOT pythonizations libraries +# CMakeLists.txt file for building ROOT pythonizations ################################################################ -set(cpp_sources - src/PyROOTModule.cxx -) - -set(libname ROOTPythonizations) - -add_library(${libname} SHARED ${cpp_sources}) - -# On Windows we can't use the stable ABI yet: it requires linking against a -# different libpython, so as long as we don't build all translation units in -# the ROOT Pythonization library with the stable ABI we should not use it. -# Free-threaded (Py_GIL_DISABLED) interpreters don't support the limited API. -if(NOT MSVC AND NOT Python3_GIL_DISABLED) - target_compile_options(${libname} PRIVATE -DPy_LIMITED_API=0x030B0000) -endif() - -# Set the suffix to '.so' and the prefix to 'lib' -set_target_properties(${libname} PROPERTIES ${ROOT_LIBRARY_PROPERTIES_NO_VERSION}) -target_link_libraries(${libname} PUBLIC Core Tree cpyrt) -if(MSVC) - set_target_properties(${libname} PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE) - set_target_properties(${libname} PROPERTIES SUFFIX ".pyd") - target_link_options(${libname} PRIVATE "-ignore:4197") -elseif(APPLE) - target_link_libraries(${libname} PUBLIC -Wl,-bind_at_load -Wl,-w -Wl,-undefined -Wl,dynamic_lookup) -else() - target_link_libraries(${libname} PUBLIC -Wl,--unresolved-symbols=ignore-all) -endif() - -# Disables warnings caused by Py_RETURN_TRUE/Py_RETURN_FALSE -if(NOT MSVC) - target_compile_options(${libname} PRIVATE -Wno-strict-aliasing) -endif() - - # Create meta-target PyROOT3 (INTERFACE library) # Export of targets are not supported for custom targets(add_custom_targets()) add_library(PyROOT INTERFACE) -target_link_libraries(PyROOT INTERFACE cppjit_backend cppjit ROOTPythonizations) - -# Define library output directories for build and install trees -set(pymoduledir_build "${localruntimedir}/ROOT") -set(pymoduledir_install "${CMAKE_INSTALL_PYTHONDIR}/ROOT") - -# To make sure that the library also ends up in the right subdirectory in the -# build directory tree. -if(MSVC) - set_target_properties(${libname} - PROPERTIES - RUNTIME_OUTPUT_DIRECTORY_DEBUG ${pymoduledir_build} - RUNTIME_OUTPUT_DIRECTORY_RELEASE ${pymoduledir_build} - RUNTIME_OUTPUT_DIRECTORY_MINSIZEREL ${pymoduledir_build} - RUNTIME_OUTPUT_DIRECTORY_RELWITHDEBINFO ${pymoduledir_build}) -else() - set_target_properties(${libname} PROPERTIES - LIBRARY_OUTPUT_DIRECTORY ${pymoduledir_build}) -endif() - -if(NOT MSVC) - # Make sure that relative RUNPATH to main ROOT libraries is always correct. - ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH(${libname} ${pymoduledir_install}) -endif() - -# Install library -install(TARGETS ${libname} EXPORT ${CMAKE_PROJECT_NAME}Exports - RUNTIME DESTINATION ${pymoduledir_install} COMPONENT libraries - LIBRARY DESTINATION ${pymoduledir_install} COMPONENT libraries - ARCHIVE DESTINATION ${pymoduledir_install} COMPONENT libraries) +target_link_libraries(PyROOT INTERFACE cppjit_backend cppjit) # For the ROOT Python package add_subdirectory(python) diff --git a/bindings/pyroot/pythonizations/python/ROOT/__init__.py b/bindings/pyroot/pythonizations/python/ROOT/__init__.py index f8aa60ad7a49d..f7c6314bf9aed 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/__init__.py +++ b/bindings/pyroot/pythonizations/python/ROOT/__init__.py @@ -42,11 +42,9 @@ def _major_minor(v): """ raise ImportError(textwrap.dedent(message)) -# The libROOTPythonizations CPython extension is in the same directory as the -# ROOT Python module, but to find the other ROOT libraries we need to also add -# the path of the ROOT library directory (only needed on Windows). For example, -# if the ROOT Python module is in $ROOTSYS/bin/ROOT/__init__.py, the libraries -# are usually in $ROOTSYS/bin. +# To find the ROOT libraries, we need to add the path of the ROOT library +# directory (only needed on Windows). For example, if the ROOT Python module is +# in $ROOTSYS/bin/ROOT/__init__.py, the libraries are usually in $ROOTSYS/bin. if "win32" in sys.platform: root_module_path = os.path.dirname(__file__) # expected to be ${CMAKE_INSTALL_PYTHONDIR}/ROOT root_install_pythondir = os.path.dirname(root_module_path) # expected to be ${CMAKE_INSTALL_PYTHONDIR} diff --git a/bindings/pyroot/pythonizations/python/ROOT/_application.py b/bindings/pyroot/pythonizations/python/ROOT/_application.py index 255636a0bffc2..cfbc40879182a 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_application.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_application.py @@ -27,11 +27,10 @@ def __init__(self, config, is_ipython): import cppyy if not cppyy.gbl.gApplication: - from ROOT.libROOTPythonizations import InstallWarningHandler - self._create_application(config.IgnoreCommandLineOptions) self._init_root_globals() - InstallWarningHandler() + # Turn ROOT warnings into Python warnings + cppyy.gbl.ROOT.Internal.SetWarningHandler(cppyy._backend.warning_handler(module="ROOT")) self._is_ipython = is_ipython @@ -117,9 +116,9 @@ def _inputhook_config(): # PyOS_InputHook-based mechanism # Point to a function which will be called when Python's interpreter prompt # is about to become idle and wait for user input from the terminal - from ROOT.libROOTPythonizations import InstallGUIEventInputHook + import cppyy - InstallGUIEventInputHook() + cppyy._backend.set_input_hook(cppyy.gbl.ROOT.Internal.ProcessGUIEvents) @staticmethod def _set_display_hook(): diff --git a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx deleted file mode 100644 index 3c7c13b446a42..0000000000000 --- a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx +++ /dev/null @@ -1,133 +0,0 @@ -// Author: Enric Tejedor CERN 06/2018 -// Original PyROOT code by Wim Lavrijsen, LBL - -/************************************************************************* - * Copyright (C) 1995-2018, Rene Brun and Fons Rademakers. * - * All rights reserved. * - * * - * For the licensing terms see $ROOTSYS/LICENSE. * - * For the list of contributors see $ROOTSYS/README/CREDITS. * - *************************************************************************/ - -// Cppyy -#include "cpyrt/API.h" - -// ROOT -#include "TError.h" -#include "TInterpreter.h" -#include "TROOT.h" -#include "TSystem.h" -#include "TVirtualPad.h" -#include "RConfigure.h" - -// Standard -#include -#include -#include -#include - -namespace PyROOT { - -PyObject *gRootModule = nullptr; - -} // namespace PyROOT - -namespace { - -/// Turns a ROOT warning into a Python warning, see ROOT::Internal::SetWarningHandler(). -bool WarningHandler(const char *location, const char *msg) -{ - PyGILState_STATE state = PyGILState_Ensure(); - // Python code can't run while an exception is pending, e.g. a previous - // warning turned into an error: decline, and ROOT prints the warning. - bool handled = !PyErr_Occurred(); - if (handled) { - // Prints or, per the warnings filters, raises: the exception stays - // pending until the call into C++ returns to Python. - PyErr_WarnExplicit(nullptr, msg, location, 0, "ROOT", nullptr); - } - PyGILState_Release(state); - return handled; -} - -int (*gPreviousInputHook)() = nullptr; - -/// Processes GUI events while the Python prompt waits for input. Runs no -/// Python code, so a Ctrl-C arriving meanwhile stays pending for the prompt. -int GUIEventInputHook() -{ - PyGILState_STATE state = PyGILState_Ensure(); - if (gPad && gPad->IsWeb()) - gPad->UpdateAsync(); - gSystem->ProcessEvents(); - PyGILState_Release(state); - - return gPreviousInputHook ? gPreviousInputHook() : 0; -} - -/// Turn ROOT warnings into Python warnings. -PyObject *InstallWarningHandler(PyObject * /*self*/, PyObject * /*args*/) -{ - ROOT::Internal::SetWarningHandler(&WarningHandler); - Py_RETURN_NONE; -} - -/// Install the PyOS_InputHook that processes GUI events, chaining to the -/// previous hook. -PyObject *InstallGUIEventInputHook(PyObject * /*self*/, PyObject * /*args*/) -{ - if (PyOS_InputHook != &GUIEventInputHook) { - gPreviousInputHook = PyOS_InputHook; - PyOS_InputHook = &GUIEventInputHook; - } - Py_RETURN_NONE; -} - -} // namespace - -// Methods offered by the interface -static PyMethodDef gPyROOTMethods[] = { - {"InstallWarningHandler", (PyCFunction)InstallWarningHandler, METH_NOARGS, - "Turn ROOT warnings into Python warnings"}, - {"InstallGUIEventInputHook", (PyCFunction)InstallGUIEventInputHook, METH_NOARGS, - "Install an input hook to process GUI events"}, - {NULL, NULL, 0, NULL}}; - -struct module_state { - PyObject *error; -}; - -#define GETSTATE(m) ((struct module_state *)PyModule_GetState(m)) - -static int rootmodule_traverse(PyObject *m, visitproc visit, void *arg) -{ - Py_VISIT(GETSTATE(m)->error); - return 0; -} - -static int rootmodule_clear(PyObject *m) -{ - Py_CLEAR(GETSTATE(m)->error); - return 0; -} - -static struct PyModuleDef moduledef = {PyModuleDef_HEAD_INIT, "libROOTPythonizations", NULL, - sizeof(struct module_state), gPyROOTMethods, NULL, - rootmodule_traverse, rootmodule_clear, NULL}; - -/// Initialization of extension module libROOTPythonizations - -extern "C" PyObject *PyInit_libROOTPythonizations() -{ - using namespace PyROOT; - - // setup PyROOT - gRootModule = PyModule_Create(&moduledef); - if (!gRootModule) - return nullptr; - - // keep gRootModule, but do not increase its reference count even as it is borrowed, - // or a self-referencing cycle would be created - - return gRootModule; -} diff --git a/bindings/pyroot/pythonizations/test/import_load_libs.py b/bindings/pyroot/pythonizations/test/import_load_libs.py index e3c42ef9d7108..2f21e0dbae486 100644 --- a/bindings/pyroot/pythonizations/test/import_load_libs.py +++ b/bindings/pyroot/pythonizations/test/import_load_libs.py @@ -60,7 +60,6 @@ class ImportLoadLibs(unittest.TestCase): # cppjit and Python libraries "libcppjit.*", "libcpyrt.*", - "libROOTPythonizations.*", "libpython.*", "libutil.*", ".*cpython.*", diff --git a/bindings/pyroot/pythonizations/test/regression_pickle_no_cppyy.py b/bindings/pyroot/pythonizations/test/regression_pickle_no_cppyy.py index 1683ba3ee517e..2208be0d12314 100644 --- a/bindings/pyroot/pythonizations/test/regression_pickle_no_cppyy.py +++ b/bindings/pyroot/pythonizations/test/regression_pickle_no_cppyy.py @@ -12,11 +12,11 @@ class RegressionPickleNoCppyy(unittest.TestCase): Reading back a pickled ROOT object in a fresh interpreter, where the cppyy backend has not been imported yet, must not crash. - Unpickling a ROOT object only imports ``ROOT.libROOTPythonizations`` (to - resolve ``_CPPInstance__expand__``), which does not initialize the cppyy - backend. Entering CPyCppyy through the public API in that state used to - leave ``gThisModule`` null, so that ``CreateScopeProxy`` crashed with a - segmentation violation when using it as a fake scope. + Unpickling a ROOT object first imports the module of the function that + reads it back. When that was a CPython extension module, which did not + initialize the cppyy backend, entering CPyCppyy through the public API + used to leave ``gThisModule`` null, so that ``CreateScopeProxy`` crashed + with a segmentation violation when using it as a fake scope. See the ROOT forum report: https://root-forum.cern.ch/t/issue-with-new-root-version-on-lxplus diff --git a/bindings/pyroot/pythonizations/test/th1.py b/bindings/pyroot/pythonizations/test/th1.py index 6d7332e5c0d0b..c16930a2f1d5c 100644 --- a/bindings/pyroot/pythonizations/test/th1.py +++ b/bindings/pyroot/pythonizations/test/th1.py @@ -75,7 +75,7 @@ def test_fit_with_warning(self): h = ROOT.TH1D("test", "test", 100, 0, 1) # Trying to fit a empty histogram will result in a warning on the C++ side which is re-raised as a Python - # warning by the ROOT CPython extension + # warning by the handler installed with ROOT::Internal::SetWarningHandler() with self.assertWarns(RuntimeWarning, msg="Fit data is empty"): h.Fit("gaus", "S")