From 7a6dad3ef553079b78dc8e2b6d0a983ba79613ff Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Wed, 19 Aug 2026 14:46:33 +0000 Subject: [PATCH 1/3] [Python] Connect Python callables to signals directly, no TPyDispatcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TQObject::Connect() now directly accepts a Python callable as the slot, e.g. button.Connect("Clicked()", on_clicked). The pythonization creates the dispatcher internally and keeps it alive for the lifetime of the connection, which can be undone with Disconnect(signal, callable). The signal arguments are forwarded to the callable, as far as its signature accepts them. The dispatcher is a minimal class generated in the interpreter for exactly the signal's argument types, holding the callable as a std::function. The conversion from the Python callable and the callback into Python are done by cppyy, with the compiled entry point captured by address inside the std::function. Consequently: * The pythonization layer no longer installs a public C++ header that interacts with the Python C API (the PyObject forward declaration in ROOT/TPyDispatcher.h). * Signals of any signature can be connected, no longer only those covered by the fixed set of TPyDispatcher::Dispatch() overloads. * The interpreter does not need to resolve any symbols from libROOTPythonizations. That requirement is what had silently broken TPyDispatcher in recent releases: the library is loaded as a Python extension module without exposing its symbols for linking, so jitting calls into it failed with unresolved symbols. The TPyDispatcher class, its sources, and the ROOT.TPyDispatcher attribute are removed. Migration: replace obj.Connect(signal, "TPyDispatcher", disp, "Dispatch()") with obj.Connect(signal, callable). The GUI tutorials are updated accordingly. ModelInspector.py in particular relied on use-after-free before: its dispatchers were method locals that were garbage collected right after Connect() returned. 🤖 Done with the help of AI --- README/ReleaseNotes/v642/index.md | 16 ++ bindings/pyroot/pythonizations/CMakeLists.txt | 18 -- .../pyroot/pythonizations/inc/TPyDispatcher.h | 136 --------- .../pythonizations/python/CMakeLists.txt | 1 + .../pythonizations/python/ROOT/_facade.py | 8 - .../python/ROOT/_pythonization/_tqobject.py | 195 +++++++++++++ .../pythonizations/src/TPyDispatcher.cxx | 266 ------------------ .../pyroot/pythonizations/test/CMakeLists.txt | 3 + .../pythonizations/test/tqobject_connect.py | 128 +++++++++ tutorials/roofit/roostats/ModelInspector.py | 19 +- tutorials/visualisation/gui/gui_simple.py | 5 +- tutorials/visualisation/gui/numberEntry.py | 5 +- 12 files changed, 353 insertions(+), 447 deletions(-) delete mode 100644 bindings/pyroot/pythonizations/inc/TPyDispatcher.h create mode 100644 bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tqobject.py delete mode 100644 bindings/pyroot/pythonizations/src/TPyDispatcher.cxx create mode 100644 bindings/pyroot/pythonizations/test/tqobject_connect.py diff --git a/README/ReleaseNotes/v642/index.md b/README/ReleaseNotes/v642/index.md index 60d82bd7af8b2..12ba6ea42dbc9 100644 --- a/README/ReleaseNotes/v642/index.md +++ b/README/ReleaseNotes/v642/index.md @@ -70,6 +70,22 @@ The following people have contributed to this new version: ## Python Interface +### Connecting Python callables to signals + +`TQObject::Connect()` now directly accepts a Python callable as the slot, for +example `button.Connect("Clicked()", on_clicked)`. The arguments emitted by the +signal are forwarded to the callable, as far as its signature accepts them, and +the connection keeps the callable alive. Use `Disconnect(signal, callable)` to +undo the connection. Signals of any signature are supported, no longer only +those covered by the `TPyDispatcher::Dispatch()` overloads. + +The `TPyDispatcher` class and its `ROOT/TPyDispatcher.h` header are removed: +it required the user to create and keep alive a dispatcher object manually, +and it was broken in recent releases anyway, since the interpreter could not +resolve its symbols from the `libROOTPythonizations` Python extension module. +Replace `obj.Connect(signal, "TPyDispatcher", disp, "Dispatch()")` with +`obj.Connect(signal, callable)`. + ## I/O ## Core diff --git a/bindings/pyroot/pythonizations/CMakeLists.txt b/bindings/pyroot/pythonizations/CMakeLists.txt index 8649bcd9ed578..04aeb31ac581a 100644 --- a/bindings/pyroot/pythonizations/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/CMakeLists.txt @@ -8,9 +8,6 @@ # CMakeLists.txt file for building ROOT pythonizations libraries ################################################################ -list(APPEND PYROOT_EXTRA_HEADERS - inc/TPyDispatcher.h) - set(cpp_sources src/PyROOTModule.cxx src/RPyROOTApplication.cxx @@ -18,15 +15,8 @@ set(cpp_sources src/TClassPyz.cxx src/TTreePyz.cxx src/CPPInstancePyz.cxx - src/TPyDispatcher.cxx - inc/TPyDispatcher.h ) -set(ROOT_headers_dir inc) - -# Copy headers inside build_dir/include/ROOT -file(COPY ${ROOT_headers_dir}/ DESTINATION ${CMAKE_BINARY_DIR}/include/ROOT) - set(libname ROOTPythonizations) add_library(${libname} SHARED ${cpp_sources}) @@ -56,9 +46,6 @@ else() target_link_libraries(${libname} PUBLIC -Wl,--unresolved-symbols=ignore-all) endif() -target_include_directories(${libname} - PUBLIC $) - # Disables warnings caused by Py_RETURN_TRUE/Py_RETURN_FALSE if(NOT MSVC) target_compile_options(${libname} PRIVATE -Wno-strict-aliasing) @@ -99,11 +86,6 @@ install(TARGETS ${libname} EXPORT ${CMAKE_PROJECT_NAME}Exports LIBRARY DESTINATION ${pymoduledir_install} COMPONENT libraries ARCHIVE DESTINATION ${pymoduledir_install} COMPONENT libraries) -# Install headers required by pythonizations -install(FILES ${PYROOT_EXTRA_HEADERS} - DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/ROOT - COMPONENT headers) - # For the ROOT Python package add_subdirectory(python) diff --git a/bindings/pyroot/pythonizations/inc/TPyDispatcher.h b/bindings/pyroot/pythonizations/inc/TPyDispatcher.h deleted file mode 100644 index 9c5786fa21140..0000000000000 --- a/bindings/pyroot/pythonizations/inc/TPyDispatcher.h +++ /dev/null @@ -1,136 +0,0 @@ -// Author: Enric Tejedor CERN 07/2020 -// Original PyROOT code by Wim Lavrijsen, LBL - -/************************************************************************* - * 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. * - *************************************************************************/ - -#ifndef ROOT_TPyDispatcher -#define ROOT_TPyDispatcher - -// ROOT -#include "TObject.h" - -class TDNDData; -class TEveDigitSet; -class TEveElement; -class TEveTrack; -class TEveWindow; -class TGFrame; -class TGListTreeItem; -class TGMdiFrame; -class TGLPhysicalShape; -class TGShutterItem; -class TGLVEntry; -class TGLViewerBase; -class TGVFileSplitter; -class TList; -class TObject; -class TPad; -class TQCommand; -class TSocket; -class TVirtualPad; - -struct Event_t; - -// Python -struct _object; -typedef _object PyObject; - -/// Dispatcher for C++ callbacks into Python code. -class TPyDispatcher : public TObject { -public: - TPyDispatcher(PyObject *callable); - TPyDispatcher(const TPyDispatcher &); - TPyDispatcher &operator=(const TPyDispatcher &); - ~TPyDispatcher() override; - -public: - PyObject *DispatchVA(const char *format = 0, ...); - PyObject *DispatchVA1(const char *clname, void *obj, const char *format, ...); - - // pre-defined dispatches, same as per TQObject::Emit(); note that - // Emit() maps exclusively to this set, so several builtin types (e.g. - // Int_t, Bool_t, Float_t, etc.) have been omitted here - PyObject *Dispatch() { return DispatchVA(0); } - PyObject *Dispatch(const char *param) { return DispatchVA("s", param); } - PyObject *Dispatch(Double_t param) { return DispatchVA("d", param); } - PyObject *Dispatch(Long_t param) { return DispatchVA("l", param); } - PyObject *Dispatch(Long64_t param) { return DispatchVA("L", param); } - - // further selection of pre-defined, existing dispatches - PyObject *Dispatch(Bool_t param) { return DispatchVA("i", param); } - PyObject *Dispatch(char *param) { return DispatchVA("s", param); } - PyObject *Dispatch(const char *text, Int_t len) { return DispatchVA("si", text, len); } - PyObject *Dispatch(Int_t param) { return DispatchVA("i", param); } - PyObject *Dispatch(Int_t x, Int_t y) { return DispatchVA("ii", x, y); } - PyObject *Dispatch(ULong_t param) { return DispatchVA("k", param); } - // ULong_t also for Handle_t (and Window_t, etc. ... ) - - PyObject *Dispatch(Event_t *event) { return DispatchVA1("Event_t", event, 0); } - PyObject *Dispatch(Event_t *event, ULong_t wid) { return DispatchVA1("Event_t", event, "k", wid); } - PyObject *Dispatch(TEveDigitSet *qs, Int_t idx) { return DispatchVA1("TEveDigitSet", qs, "i", idx); } - PyObject *Dispatch(TEveElement *el) { return DispatchVA1("TEveElement", el, 0); } - PyObject *Dispatch(TEveTrack *et) { return DispatchVA1("TEveTrack", et, 0); } - PyObject *Dispatch(TEveWindow *window) { return DispatchVA1("TEveWindow", window, 0); } - PyObject *Dispatch(TGFrame *frame) { return DispatchVA1("TGFrame", frame, 0); } - PyObject *Dispatch(TGFrame *frame, Int_t btn) { return DispatchVA1("TGFrame", frame, "i", btn); } - PyObject *Dispatch(TGFrame *frame, Int_t btn, Int_t x, Int_t y) - { - return DispatchVA1("TGFrame", frame, "iii", btn, x, y); - } - PyObject *Dispatch(TGFrame *frame, UInt_t keysym, UInt_t mask) - { - return DispatchVA1("TGFrame", frame, "II", keysym, mask); - } - PyObject *Dispatch(TGListTreeItem *entry) { return DispatchVA1("TGListTreeItem", entry, 0); } - PyObject *Dispatch(TGListTreeItem *entry, UInt_t mask) { return DispatchVA1("TGListTreeItem", entry, "I", mask); } - PyObject *Dispatch(TGListTreeItem *entry, UInt_t keysym, UInt_t mask) - { - return DispatchVA1("TGListTreeItem", entry, "II", keysym, mask); - } - PyObject *Dispatch(TGListTreeItem *entry, Int_t btn) { return DispatchVA1("TGListTreeItem", entry, "i", btn); } - PyObject *Dispatch(TGListTreeItem *entry, Int_t btn, Int_t x, Int_t y) - { - return DispatchVA1("TGListTreeItem", entry, "iii", btn, x, y); - } - PyObject *Dispatch(TGLVEntry *entry, Int_t btn) { return DispatchVA1("TGLVEntry", entry, "i", btn); } - PyObject *Dispatch(TGLVEntry *entry, Int_t btn, Int_t x, Int_t y) - { - return DispatchVA1("TGLVEntry", entry, "iii", btn, x, y); - } - PyObject *Dispatch(TGLViewerBase *viewer) { return DispatchVA1("TGLViewerBase", viewer, 0); } - PyObject *Dispatch(TGLPhysicalShape *shape) { return DispatchVA1("TGLPhysicalShape", shape, 0); } - PyObject *Dispatch(TGLPhysicalShape *shape, UInt_t u1, UInt_t u2) - { - return DispatchVA1("TGLPhysicalShape", shape, "II", u1, u2); - } - PyObject *Dispatch(TGMdiFrame *frame) { return DispatchVA1("TGMdiFrame", frame, 0); } - PyObject *Dispatch(TGShutterItem *item) { return DispatchVA1("TGShutterItem", item, 0); } - PyObject *Dispatch(TGVFileSplitter *frame) { return DispatchVA1("TGVFileSplitter", frame, 0); } - PyObject *Dispatch(TList *objs) { return DispatchVA1("TList", objs, 0); } - PyObject *Dispatch(TObject *obj) { return DispatchVA1("TObject", obj, 0); } - PyObject *Dispatch(TObject *obj, Bool_t check) { return DispatchVA1("TObject", obj, "i", check); } - PyObject *Dispatch(TObject *obj, UInt_t state) { return DispatchVA1("TObject", obj, "I", state); } - PyObject *Dispatch(TObject *obj, UInt_t button, UInt_t state) - { - return DispatchVA1("TObject", obj, "II", button, state); - } - PyObject *Dispatch(TSocket *sock) { return DispatchVA1("TSocket", sock, 0); } - PyObject *Dispatch(TVirtualPad *pad) { return DispatchVA1("TVirtualPad", pad, 0); } - - PyObject *Dispatch(TPad *selpad, TObject *selected, Int_t event); - PyObject *Dispatch(Int_t event, Int_t x, Int_t y, TObject *selected); - PyObject *Dispatch(TVirtualPad *pad, TObject *obj, Int_t event); - PyObject *Dispatch(TGListTreeItem *item, TDNDData *data); - PyObject *Dispatch(const char *name, const TList *attr); - -private: - PyObject *fCallable; ///) method + forwards to a wrapped Python callable. The class is generated in the + interpreter, which is what allows connecting a TQObject signal to it by + class name; it holds the callable as a std::function, whose conversion + from a Python callable and calling back into Python are done by cppyy.""" + import cppyy + + proto = ", ".join(arg_types) + klass = _dispatcher_classes.get(proto) + if klass is None: + name = "TPyDispatcher_{}".format(len(_dispatcher_classes)) + params = ", ".join("{} a{}".format(t, i) for i, t in enumerate(arg_types)) + forwarded = ", ".join("a{}".format(i) for i in range(len(arg_types))) + cppyy.cppdef( + """ +#include +#include + +namespace PyROOT {{ + +class {name} {{ +public: + {name}(std::function callable) : fCallable(std::move(callable)) {{}} + void Dispatch({params}) {{ fCallable({forwarded}); }} + +private: + std::function fCallable; +}}; + +}} // namespace PyROOT +""".format(name=name, proto=proto, params=params, forwarded=forwarded) + ) + klass = getattr(cppyy.gbl.PyROOT, name) + _dispatcher_classes[proto] = klass + return klass + + +def _connection_key(sender, signal, callable_): + import cppyy + + return (cppyy.addressof(sender), signal.replace(" ", ""), callable_) + + +def _max_accepted_args(callable_): + """Maximum number of positional arguments the callable accepts, or None if + unbounded or undeterminable.""" + try: + signature = inspect.signature(callable_) + except (TypeError, ValueError): + return None + nmax = 0 + for par in signature.parameters.values(): + if par.kind in (par.POSITIONAL_ONLY, par.POSITIONAL_OR_KEYWORD): + nmax += 1 + elif par.kind == par.VAR_POSITIONAL: + return None + return nmax + + +def _signal_arg_types(signal, callable_): + """The argument types the signal emits, truncated to what the callable + accepts.""" + lpar = signal.find("(") + rpar = signal.rfind(")") + if lpar < 0 or rpar < lpar: + raise ValueError('signal "{}" lacks the argument list, e.g. "Clicked()"'.format(signal)) + proto = signal[lpar + 1 : rpar].strip() + arg_types = [a.strip() for a in proto.split(",")] if proto else [] + nmax = _max_accepted_args(callable_) + return arg_types if nmax is None else arg_types[:nmax] + + +def _print_errors(callable_): + """Print exceptions from the callable instead of letting them escape: the + slot is invoked from C++ signal emission, which exceptions cannot safely + propagate through.""" + + def wrapper(*args): + try: + callable_(*args) + except Exception: + traceback.print_exc() + + return wrapper + + +def _is_new_style_args(args, kwargs): + return not kwargs and len(args) == 2 and not isinstance(args[1], str) and callable(args[1]) + + +def _TQObject_Connect(self, *args, **kwargs): + if isinstance(self, str): + # Static overload connecting by sender class name, called unbound, + # e.g. TQObject.Connect("TGButton", "Clicked()", ...) + import cppyy + + return cppyy.gbl.TQObject._OriginalConnect(self, *args, **kwargs) + if _is_new_style_args(args, kwargs): + signal, callable_ = args + arg_types = _signal_arg_types(signal, callable_) + wrapper = _print_errors(callable_) + dispatcher = _dispatcher_class(arg_types)(wrapper) + # The std::function holds only a borrowed reference to the Python + # callable, so tie the wrapper's lifetime to the dispatcher + dispatcher._callable = wrapper + result = self._OriginalConnect( + signal, type(dispatcher).__cpp_name__, dispatcher, "Dispatch({})".format(", ".join(arg_types)) + ) + if result: + _dispatchers.setdefault(_connection_key(self, signal, callable_), []).append(dispatcher) + return result + return self._OriginalConnect(*args, **kwargs) + + +def _TQObject_Disconnect(self, *args, **kwargs): + if isinstance(self, str): + import cppyy + + return cppyy.gbl.TQObject._OriginalDisconnect(self, *args, **kwargs) + if _is_new_style_args(args, kwargs): + signal, callable_ = args + dispatchers = _dispatchers.pop(_connection_key(self, signal, callable_), None) + if not dispatchers: + return False + result = False + for dispatcher in dispatchers: + result = self._OriginalDisconnect(signal, dispatcher) or result + return result + return self._OriginalDisconnect(*args, **kwargs) + + +@pythonization("TQObject") +def pythonize_tqobject(klass): + klass._OriginalConnect = klass.Connect + klass.Connect = _TQObject_Connect + klass._OriginalDisconnect = klass.Disconnect + klass.Disconnect = _TQObject_Disconnect diff --git a/bindings/pyroot/pythonizations/src/TPyDispatcher.cxx b/bindings/pyroot/pythonizations/src/TPyDispatcher.cxx deleted file mode 100644 index fa5ba950528f9..0000000000000 --- a/bindings/pyroot/pythonizations/src/TPyDispatcher.cxx +++ /dev/null @@ -1,266 +0,0 @@ -// Author: Enric Tejedor CERN 07/2020 -// Original PyROOT code by Wim Lavrijsen, LBL - -/************************************************************************* - * 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 "TPyDispatcher.h" - -// Bindings -#include "CPyCppyy/API.h" - -// ROOT -#include "TClass.h" -#include "TObject.h" - -// Standard -#include - -//______________________________________________________________________________ -// Python callback dispatcher -// ========================== -// -// The TPyDispatcher class acts as a functor that can be used for TFn's and GUIs -// to install callbacks from Cling. - -//- constructors/destructor -------------------------------------------------- -TPyDispatcher::TPyDispatcher(PyObject *callable) : fCallable(0) -{ - // Construct a TPyDispatcher from a callable python object. Applies python - // object reference counting. - Py_XINCREF(callable); - fCallable = callable; -} - -//////////////////////////////////////////////////////////////////////////////// -/// Copy constructor. Applies python object reference counting. - -TPyDispatcher::TPyDispatcher(const TPyDispatcher &other) : TObject(other) -{ - Py_XINCREF(other.fCallable); - fCallable = other.fCallable; -} - -//////////////////////////////////////////////////////////////////////////////// -/// Assignment operator. Applies python object reference counting. - -TPyDispatcher &TPyDispatcher::operator=(const TPyDispatcher &other) -{ - if (this != &other) { - this->TObject::operator=(other); - - Py_DecRef(fCallable); - Py_XINCREF(other.fCallable); - fCallable = other.fCallable; - } - - return *this; -} - -//////////////////////////////////////////////////////////////////////////////// -/// Destructor. Reference counting for the held python object is in effect. - -TPyDispatcher::~TPyDispatcher() -{ - Py_DecRef(fCallable); -} - -//- public members ----------------------------------------------------------- -PyObject *TPyDispatcher::DispatchVA(const char *format, ...) -{ - // Dispatch the arguments to the held callable python object, using format to - // interpret the types of the arguments. Note that format is in python style, - // not in C printf style. See: https://docs.python.org/2/c-api/arg.html . - PyObject *args = 0; - - if (format) { - va_list va; - va_start(va, format); - - args = Py_VaBuildValue((char *)format, va); - - va_end(va); - - if (!args) { - PyErr_Print(); - return 0; - } - - if (!PyTuple_Check(args)) { // if only one arg ... - PyObject *t = PyTuple_New(1); - PyTuple_SetItem(t, 0, args); - args = t; - } - } - - PyObject *result = PyObject_CallObject(fCallable, args); - Py_DecRef(args); - - if (!result) { - PyErr_Print(); - return 0; - } - - return result; -} - -//////////////////////////////////////////////////////////////////////////////// - -PyObject *TPyDispatcher::DispatchVA1(const char *clname, void *obj, const char *format, ...) -{ - PyObject *pyobj = CPyCppyy::Instance_FromVoidPtr(obj, clname); - if (!pyobj) { - PyErr_Print(); - return 0; - } - - PyObject *args = 0; - - if (format) { - va_list va; - va_start(va, format); - - args = Py_VaBuildValue((char *)format, va); - - va_end(va); - - if (!args) { - PyErr_Print(); - return 0; - } - - if (!PyTuple_Check(args)) { // if only one arg ... - PyObject *t = PyTuple_New(2); - PyTuple_SetItem(t, 0, pyobj); - PyTuple_SetItem(t, 1, args); - args = t; - } else { - PyObject *t = PyTuple_New(PyTuple_Size(args) + 1); - PyTuple_SetItem(t, 0, pyobj); - for (int i = 0; i < PyTuple_Size(args); i++) { - PyObject *item = PyTuple_GetItem(args, i); - Py_IncRef(item); - PyTuple_SetItem(t, i + 1, item); - } - Py_DecRef(args); - args = t; - } - } else { - args = PyTuple_New(1); - PyTuple_SetItem(args, 0, pyobj); - } - - PyObject *result = PyObject_CallObject(fCallable, args); - Py_DecRef(args); - - if (!result) { - PyErr_Print(); - return 0; - } - - return result; -} - -//////////////////////////////////////////////////////////////////////////////// - -PyObject *TPyDispatcher::Dispatch(TPad *selpad, TObject *selected, Int_t event) -{ - PyObject *args = PyTuple_New(3); - PyTuple_SetItem(args, 0, CPyCppyy::Instance_FromVoidPtr(selpad, "TPad")); - PyTuple_SetItem(args, 1, CPyCppyy::Instance_FromVoidPtr(selected, "TObject")); - PyTuple_SetItem(args, 2, PyLong_FromLong(event)); - - PyObject *result = PyObject_CallObject(fCallable, args); - Py_DecRef(args); - - if (!result) { - PyErr_Print(); - return 0; - } - - return result; -} - -//////////////////////////////////////////////////////////////////////////////// - -PyObject *TPyDispatcher::Dispatch(Int_t event, Int_t x, Int_t y, TObject *selected) -{ - PyObject *args = PyTuple_New(4); - PyTuple_SetItem(args, 0, PyLong_FromLong(event)); - PyTuple_SetItem(args, 1, PyLong_FromLong(x)); - PyTuple_SetItem(args, 2, PyLong_FromLong(y)); - PyTuple_SetItem(args, 3, CPyCppyy::Instance_FromVoidPtr(selected, "TObject")); - - PyObject *result = PyObject_CallObject(fCallable, args); - Py_DecRef(args); - - if (!result) { - PyErr_Print(); - return 0; - } - - return result; -} - -//////////////////////////////////////////////////////////////////////////////// - -PyObject *TPyDispatcher::Dispatch(TVirtualPad *pad, TObject *obj, Int_t event) -{ - PyObject *args = PyTuple_New(3); - PyTuple_SetItem(args, 0, CPyCppyy::Instance_FromVoidPtr(pad, "TVirtualPad")); - PyTuple_SetItem(args, 1, CPyCppyy::Instance_FromVoidPtr(obj, "TObject")); - PyTuple_SetItem(args, 2, PyLong_FromLong(event)); - - PyObject *result = PyObject_CallObject(fCallable, args); - Py_DecRef(args); - - if (!result) { - PyErr_Print(); - return 0; - } - - return result; -} - -//////////////////////////////////////////////////////////////////////////////// - -PyObject *TPyDispatcher::Dispatch(TGListTreeItem *item, TDNDData *data) -{ - PyObject *args = PyTuple_New(2); - PyTuple_SetItem(args, 0, CPyCppyy::Instance_FromVoidPtr(item, "TGListTreeItem")); - PyTuple_SetItem(args, 1, CPyCppyy::Instance_FromVoidPtr(data, "TDNDData")); - - PyObject *result = PyObject_CallObject(fCallable, args); - Py_DecRef(args); - - if (!result) { - PyErr_Print(); - return 0; - } - - return result; -} - -//////////////////////////////////////////////////////////////////////////////// - -PyObject *TPyDispatcher::Dispatch(const char *name, const TList *attr) -{ - PyObject *args = PyTuple_New(2); - PyTuple_SetItem(args, 0, PyBytes_FromString(name)); - PyTuple_SetItem(args, 1, CPyCppyy::Instance_FromVoidPtr((void *)attr, "TList")); - - PyObject *result = PyObject_CallObject(fCallable, args); - Py_DecRef(args); - - if (!result) { - PyErr_Print(); - return 0; - } - - return result; -} diff --git a/bindings/pyroot/pythonizations/test/CMakeLists.txt b/bindings/pyroot/pythonizations/test/CMakeLists.txt index a4baa0c51b9c7..fbee1dd8fb1bc 100644 --- a/bindings/pyroot/pythonizations/test/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/test/CMakeLists.txt @@ -46,6 +46,9 @@ ROOT_ADD_PYUNITTEST(pyroot_pyz_ttree_branch ttree_branch.py PYTHON_DEPS numpy) # TColor-related pythonizations (regression_20018 lives in tcolor.py) ROOT_ADD_PYUNITTEST(pyroot_pyz_tcolor tcolor.py) +# TQObject::Connect with Python callables +ROOT_ADD_PYUNITTEST(pyroot_pyz_tqobject_connect tqobject_connect.py) + # TH1 and subclasses pythonizations ROOT_ADD_PYUNITTEST(pyroot_pyz_th1 th1.py) # The above tests a deadlock. It should complete in about 1s. If we don't reduce the timeout, we need to wait 1500 s. diff --git a/bindings/pyroot/pythonizations/test/tqobject_connect.py b/bindings/pyroot/pythonizations/test/tqobject_connect.py new file mode 100644 index 0000000000000..d1dad7c91d605 --- /dev/null +++ b/bindings/pyroot/pythonizations/test/tqobject_connect.py @@ -0,0 +1,128 @@ +import gc +import unittest + +import ROOT + +ROOT.gInterpreter.Declare(""" +class PyTestEmitter : public TQObject { +public: + void Go(Int_t i) { Emit("Go(Int_t)", i); } + void Go3(Int_t i, Int_t j, Int_t k) { EmitVA("Go3(Int_t,Int_t,Int_t)", 3, i, j, k); } + void Ping() { Emit("Ping()"); } + // Connect() determines the sender class via IsA(); compiled classes get + // this override from their ClassDef macro + TClass *IsA() const override { return TClass::GetClass("PyTestEmitter"); } +}; +""") + + +class TQObjectConnect(unittest.TestCase): + """ + Test the pythonization of TQObject::Connect and Disconnect that directly + accepts Python callables as slots. + """ + + def test_connect_no_args(self): + emitter = ROOT.PyTestEmitter() + calls = [] + self.assertTrue(emitter.Connect("Ping()", lambda: calls.append(1))) + emitter.Ping() + self.assertEqual(calls, [1]) + + def test_connect_forwards_signal_args(self): + emitter = ROOT.PyTestEmitter() + calls = [] + self.assertTrue(emitter.Connect("Go(Int_t)", calls.append)) + emitter.Go(42) + emitter.Go(7) + self.assertEqual(calls, [42, 7]) + + def test_connect_callable_with_fewer_args(self): + # A callable accepting fewer arguments than the signal emits is called + # with the arguments it can accept + emitter = ROOT.PyTestEmitter() + calls = [] + + def no_args_slot(): + calls.append("called") + + self.assertTrue(emitter.Connect("Go(Int_t)", no_args_slot)) + emitter.Go(42) + self.assertEqual(calls, ["called"]) + + def test_disconnect(self): + emitter = ROOT.PyTestEmitter() + calls = [] + + def slot(i): + calls.append(i) + + self.assertTrue(emitter.Connect("Go(Int_t)", slot)) + emitter.Go(1) + self.assertTrue(emitter.Disconnect("Go(Int_t)", slot)) + emitter.Go(2) + self.assertEqual(calls, [1]) + # Disconnecting a callable that is not connected returns False + self.assertFalse(emitter.Disconnect("Go(Int_t)", slot)) + + def test_bound_method_slot(self): + emitter = ROOT.PyTestEmitter() + + class Receiver: + def __init__(self): + self.calls = [] + + def on_go(self, i): + self.calls.append(i) + + receiver = Receiver() + self.assertTrue(emitter.Connect("Go(Int_t)", receiver.on_go)) + emitter.Go(3) + self.assertEqual(receiver.calls, [3]) + # Bound methods are recreated on each attribute access, so Disconnect + # must still find the connection + self.assertTrue(emitter.Disconnect("Go(Int_t)", receiver.on_go)) + emitter.Go(4) + self.assertEqual(receiver.calls, [3]) + + def test_connection_keeps_callable_alive(self): + emitter = ROOT.PyTestEmitter() + calls = [] + + def make_slot(): + return lambda i: calls.append(i) + + emitter.Connect("Go(Int_t)", make_slot()) + gc.collect() + emitter.Go(5) + self.assertEqual(calls, [5]) + + def test_multi_arg_signal(self): + emitter = ROOT.PyTestEmitter() + calls = [] + + def slot(i, j, k): + calls.append((i, j, k)) + + self.assertTrue(emitter.Connect("Go3(Int_t,Int_t,Int_t)", slot)) + emitter.Go3(1, 2, 3) + self.assertEqual(calls, [(1, 2, 3)]) + + def test_exception_in_slot(self): + # An exception raised in the slot is printed and does not propagate + # through the C++ signal emission + emitter = ROOT.PyTestEmitter() + calls = [] + + def raising_slot(i): + calls.append(i) + raise RuntimeError("problem in slot") + + self.assertTrue(emitter.Connect("Go(Int_t)", raising_slot)) + emitter.Go(1) + emitter.Go(2) + self.assertEqual(calls, [1, 2]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tutorials/roofit/roostats/ModelInspector.py b/tutorials/roofit/roostats/ModelInspector.py index 6e51a252eec0b..55ae48ed839a7 100644 --- a/tutorials/roofit/roostats/ModelInspector.py +++ b/tutorials/roofit/roostats/ModelInspector.py @@ -520,25 +520,21 @@ def __init__(self, w, mc, data): self.fHframe2 = ROOT.TGHorizontalFrame(self, 0, 0, 0) - dp_DoFit = ROOT.TPyDispatcher(self.DoFit) self.fFitButton = ROOT.TGTextButton(self.fHframe2, "&Fit") self.fFitButton.SetFont("Helvetica") - self.fFitButton.Connect("Clicked()", "TPyDispatcher", dp_DoFit, "Dispatch()") + self.fFitButton.Connect("Clicked()", self.DoFit) - dp_DoExit = ROOT.TPyDispatcher(self.DoExit) self.fExitButton = ROOT.TGTextButton(self.fHframe2, "&Exit") self.fExitButton.SetFont("Helvetica") - # self.fExitButton.Connect( "Clicked()", "TPyDispatcher", dp_DoExit , "Dispatch()") + # self.fExitButton.Connect("Clicked()", self.DoExit) # doesn't work properly. Break segmentation violation. Full crash. self.fExitButton.SetCommand('TPython::Exec( "raise SystemExit" )') - # dp_CloseWindow = TPyDispatcher( self.CloseWindow) - # self.Connect("CloseWindow()", "TPyDispatcher", dp_CloseWindow, "Dispatch()") + # self.Connect("CloseWindow()", self.CloseWindow) self.DontCallClose() - dp_HandleButtons = ROOT.TPyDispatcher(self.HandleButtons) - self.fCheck1.Connect("Clicked()", "TPyDispatcher", dp_HandleButtons, "Dispatch()") - self.fCheck2.Connect("Clicked()", "TPyDispatcher", dp_HandleButtons, "Dispatch()") + self.fCheck1.Connect("Clicked()", self.HandleButtons) + self.fCheck2.Connect("Clicked()", self.HandleButtons) self.fHframe2.Resize(100, 25) @@ -593,9 +589,8 @@ def __init__(self, w, mc, data): False, ) - dp_DoSlider = ROOT.TPyDispatcher(self.DoSlider) - hsliderk.Connect("PointerPositionChanged()", "TPyDispatcher", dp_DoSlider, "Dispatch()") - hsliderk.Connect("PositionChanged()", "TPyDispatcher", dp_DoSlider, "Dispatch()") + hsliderk.Connect("PointerPositionChanged()", self.DoSlider) + hsliderk.Connect("PositionChanged()", self.DoSlider) hsliderk.SetRange(param.getMin(), param.getMax()) hframek.Resize(200, 25) diff --git a/tutorials/visualisation/gui/gui_simple.py b/tutorials/visualisation/gui/gui_simple.py index 654e5159f9cbb..56ddc11148f49 100644 --- a/tutorials/visualisation/gui/gui_simple.py +++ b/tutorials/visualisation/gui/gui_simple.py @@ -32,9 +32,6 @@ def MyDraw(): tpygaus.Draw() ROOT.gPad.Update() -m = ROOT.TPyDispatcher( MyDraw ) - - class pMainFrame( ROOT.TGMainFrame ): def __init__( self, parent, width, height ): ROOT.TGMainFrame.__init__( self, parent, width, height ) @@ -44,7 +41,7 @@ def __init__( self, parent, width, height ): self.ButtonsFrame = ROOT.TGHorizontalFrame( self, 200, 40 ) self.DrawButton = ROOT.TGTextButton( self.ButtonsFrame, '&Draw', 10 ) - self.DrawButton.Connect( 'Clicked()', "TPyDispatcher", m, 'Dispatch()' ) + self.DrawButton.Connect( 'Clicked()', MyDraw ) self.ButtonsFrame.AddFrame( self.DrawButton, ROOT.TGLayoutHints() ) self.ExitButton = ROOT.TGTextButton( self.ButtonsFrame, '&Exit', 20 ) diff --git a/tutorials/visualisation/gui/numberEntry.py b/tutorials/visualisation/gui/numberEntry.py index 42b21101da857..94c82815a9952 100644 --- a/tutorials/visualisation/gui/numberEntry.py +++ b/tutorials/visualisation/gui/numberEntry.py @@ -25,9 +25,8 @@ def __init__(self, parent, width, height): ROOT.TGNumberFormat.kNEANonNegative, ROOT.TGNumberFormat.kNELLimitMinMax, 0, 99999) - self.fLabelDispatch = ROOT.TPyDispatcher(self.DoSetlabel) - self.fNumber.Connect("ValueSet(Long_t)", "TPyDispatcher", self.fLabelDispatch, "Dispatch()") - self.fNumber.GetNumberEntry().Connect("ReturnPressed()", "TPyDispatcher", self.fLabelDispatch, "Dispatch()") + self.fNumber.Connect("ValueSet(Long_t)", self.DoSetlabel) + self.fNumber.GetNumberEntry().Connect("ReturnPressed()", self.DoSetlabel) self.AddFrame(self.fNumber, ROOT.TGLayoutHints(ROOT.kLHintsTop | ROOT.kLHintsLeft, 5, 5, 5, 5)) self.fGframe = ROOT.TGGroupFrame(self, "Value") self.fLabel = ROOT.TGLabel(self.fGframe, "No input.") From 31466b8958fde7308e1eb93bc9e90cbe4ec94f5a Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Wed, 19 Aug 2026 15:22:37 +0000 Subject: [PATCH 2/3] [tutorials] Fix latent bugs in ModelInspector.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Python version of this tutorial had two bugs that went unnoticed because its GUI callbacks never actually fired (signal connections through TPyDispatcher silently failed to resolve their symbols) and the tutorial is excluded from CI: * The per-parameter widgets (frame, label, slider) were only stored in C++ containers holding raw pointers, so Python garbage-collected the proxies on each loop iteration and deleted the C++ widgets under the parent frame, crashing e.g. in MapSubwindows(). * The sliders were connected to DoSlider before their positions were initialized. Setting the position emits the connected signals, so DoSlider ran on the half-constructed GUI and crashed. Verified under Xvfb: the tutorial now runs to completion, and clicking the fit button and checkboxes programmatically runs DoFit/HandleButtons including the final DoSlider update. 🤖 Done with the help of AI --- tutorials/roofit/roostats/ModelInspector.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tutorials/roofit/roostats/ModelInspector.py b/tutorials/roofit/roostats/ModelInspector.py index 55ae48ed839a7..3a19b12740eab 100644 --- a/tutorials/roofit/roostats/ModelInspector.py +++ b/tutorials/roofit/roostats/ModelInspector.py @@ -567,6 +567,11 @@ def __init__(self, w, mc, data): # And that's it! # Obviously, the parent of other subframes is now fVFrame instead of "self"... + # The C++ containers below (TList, std::map) store only raw pointers, + # so keep the Python proxies of the per-parameter widgets referenced: + # otherwise Python garbage-collects them and deletes the C++ widgets + self.fWidgets = [] + # while (param := it.Next()): #unnecessary for param in parameters: print(f"Adding Slider for ", param.GetName()) @@ -589,8 +594,6 @@ def __init__(self, w, mc, data): False, ) - hsliderk.Connect("PointerPositionChanged()", self.DoSlider) - hsliderk.Connect("PositionChanged()", self.DoSlider) hsliderk.SetRange(param.getMin(), param.getMax()) hframek.Resize(200, 25) @@ -600,11 +603,18 @@ def __init__(self, w, mc, data): hsliderk.SetPosition(param.getVal() - param.getError(), param.getVal() + param.getError()) hsliderk.SetPointerPosition(param.getVal()) + # Connect only after initializing the slider: setting the position + # emits the signals, and DoSlider must not run on the + # half-constructed GUI + hsliderk.Connect("PointerPositionChanged()", self.DoSlider) + hsliderk.Connect("PositionChanged()", self.DoSlider) + hframek.AddFrame(hlabel, self.fBly) # hframek.AddFrame(hsliderk, self.fBly) # self.fVFrame.AddFrame(hframek, self.fBly) self.fSliderMap[hsliderk] = param.GetName() self.fLabelMap[hsliderk] = hlabel + self.fWidgets += [hframek, hlabel, hsliderk] # Set main frame name, map sub windows (buttons), initialize layout # algorithm via Resize() and map main frame From 8fcd8cdb2b537e2294476ef9998891c2a92c1942 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Wed, 19 Aug 2026 19:04:16 +0200 Subject: [PATCH 3/3] [tutorials] Apply linter suggestions to ModelInspector.py and gui_simple.py --- tutorials/roofit/roostats/ModelInspector.py | 24 +++--- tutorials/visualisation/gui/gui_simple.py | 86 +++++++++++---------- 2 files changed, 56 insertions(+), 54 deletions(-) diff --git a/tutorials/roofit/roostats/ModelInspector.py b/tutorials/roofit/roostats/ModelInspector.py index 3a19b12740eab..ff77adbbe0304 100644 --- a/tutorials/roofit/roostats/ModelInspector.py +++ b/tutorials/roofit/roostats/ModelInspector.py @@ -40,10 +40,10 @@ # \author Kyle Cranmer (C++ version), and P. P. (Python translation) import sys -import ROOT - from enum import Enum +import ROOT + class ETestCommandIdentifiers(Enum): HId1 = 1 @@ -482,13 +482,13 @@ def __init__(self, w, mc, data): numCats = 1 # if (strcmp(fMC.GetPdf().ClassName(), "RooSimultaneous") == 0) : non-pythonic syntax if self.fMC.GetPdf().ClassName() == "RooSimultaneous": # simple, pythonic syntax - print(f"Is a simultaneous PDF") + print("Is a simultaneous PDF") simPdf = self.fMC.GetPdf() channelCat = simPdf.indexCat() print(f" with {channelCat.numTypes()} categories") numCats = channelCat.numTypes() else: - print(f"Is not a simultaneous PDF") + print("Is not a simultaneous PDF") self.fFitRes = ROOT.nullptr self.SetCleanup(ROOT.kDeepCleanup) @@ -574,7 +574,7 @@ def __init__(self, w, mc, data): # while (param := it.Next()): #unnecessary for param in parameters: - print(f"Adding Slider for ", param.GetName()) + print("Adding Slider for ", param.GetName()) hframek = ROOT.TGHorizontalFrame(self.fVFrame, 0, 0, 0) hlabel = ROOT.TGLabel( @@ -640,12 +640,12 @@ def ModelInspector(infile="", workspaceName="combined", modelConfigName="ModelCo # if file does not exists generate with histfactory if not fileExist: # Normally this would be run on the command line - print(f"will run standard hist2workspace example") + print("will run standard hist2workspace example") ROOT.gROOT.ProcessLine(".! prepareHistFactory .") ROOT.gROOT.ProcessLine(".! hist2workspace config/example.xml") - print(f"\n\n---------------------") - print(f"Done creating example input") - print(f"---------------------\n\n") + print("\n\n---------------------") + print("Done creating example input") + print("---------------------\n\n") else: filename = infile @@ -653,12 +653,10 @@ def ModelInspector(infile="", workspaceName="combined", modelConfigName="ModelCo # Bad behaviour of variable, workspace, modelconfig. They get unset after its first call(being whatever) # if we declare pointer to the file, workspace, modelconfig, so everything seems to work-out fine. Declare = ROOT.gInterpreter.Declare - Declare( - """using namespace std; + Declare("""using namespace std; using namespace RooFit; using namespace RooStats; - """ - ) + """) ################################################## # Try to open the file: # Not to use: file = TFile.Open(filename, "READ" ) diff --git a/tutorials/visualisation/gui/gui_simple.py b/tutorials/visualisation/gui/gui_simple.py index 56ddc11148f49..4bbaba6714eaa 100644 --- a/tutorials/visualisation/gui/gui_simple.py +++ b/tutorials/visualisation/gui/gui_simple.py @@ -5,59 +5,63 @@ ## \macro_code ## ## \author Wim Lavrijsen -from __future__ import print_function -import os, sys, ROOT +import ROOT -def pygaus( x, par ): - import math - if (par[2] != 0.0): - arg1 = (x[0]-par[1])/par[2] - arg2 = (0.01*0.39894228)/par[2] - arg3 = par[0]/(1+par[3]) - gauss = arg3*arg2*math.exp(-0.5*arg1*arg1) - else: - print('returning 0') - gauss = 0. - return gauss +def pygaus(x, par): + import math + + if par[2] != 0.0: + arg1 = (x[0] - par[1]) / par[2] + arg2 = (0.01 * 0.39894228) / par[2] + arg3 = par[0] / (1 + par[3]) + + gauss = arg3 * arg2 * math.exp(-0.5 * arg1 * arg1) + else: + print("returning 0") + gauss = 0.0 + return gauss + + +tpygaus = ROOT.TF1("pygaus", pygaus, -4, 4, 4) +tpygaus.SetParameters(1.0, 0.0, 1.0) -tpygaus = ROOT.TF1( 'pygaus', pygaus, -4, 4, 4 ) -tpygaus.SetParameters( 1., 0., 1. ) def MyDraw(): - btn = ROOT.BindObject( ROOT.gTQSender, ROOT.TGTextButton ) - if btn.WidgetId() == 10: - global tpygaus, window - tpygaus.Draw() - ROOT.gPad.Update() + btn = ROOT.BindObject(ROOT.gTQSender, ROOT.TGTextButton) + if btn.WidgetId() == 10: + global tpygaus, window + tpygaus.Draw() + ROOT.gPad.Update() + -class pMainFrame( ROOT.TGMainFrame ): - def __init__( self, parent, width, height ): - ROOT.TGMainFrame.__init__( self, parent, width, height ) +class pMainFrame(ROOT.TGMainFrame): + def __init__(self, parent, width, height): + ROOT.TGMainFrame.__init__(self, parent, width, height) - self.Canvas = ROOT.TRootEmbeddedCanvas( 'Canvas', self, 200, 200 ) - self.AddFrame( self.Canvas, ROOT.TGLayoutHints(ROOT.kLHintsExpandX | ROOT.kLHintsExpandY) ) - self.ButtonsFrame = ROOT.TGHorizontalFrame( self, 200, 40 ) + self.Canvas = ROOT.TRootEmbeddedCanvas("Canvas", self, 200, 200) + self.AddFrame(self.Canvas, ROOT.TGLayoutHints(ROOT.kLHintsExpandX | ROOT.kLHintsExpandY)) + self.ButtonsFrame = ROOT.TGHorizontalFrame(self, 200, 40) - self.DrawButton = ROOT.TGTextButton( self.ButtonsFrame, '&Draw', 10 ) - self.DrawButton.Connect( 'Clicked()', MyDraw ) - self.ButtonsFrame.AddFrame( self.DrawButton, ROOT.TGLayoutHints() ) + self.DrawButton = ROOT.TGTextButton(self.ButtonsFrame, "&Draw", 10) + self.DrawButton.Connect("Clicked()", MyDraw) + self.ButtonsFrame.AddFrame(self.DrawButton, ROOT.TGLayoutHints()) - self.ExitButton = ROOT.TGTextButton( self.ButtonsFrame, '&Exit', 20 ) - self.ExitButton.SetCommand( 'TPython::Exec( "raise SystemExit" )' ) - self.ButtonsFrame.AddFrame( self.ExitButton, ROOT.TGLayoutHints() ) + self.ExitButton = ROOT.TGTextButton(self.ButtonsFrame, "&Exit", 20) + self.ExitButton.SetCommand('TPython::Exec( "raise SystemExit" )') + self.ButtonsFrame.AddFrame(self.ExitButton, ROOT.TGLayoutHints()) - self.AddFrame( self.ButtonsFrame, ROOT.TGLayoutHints() ) + self.AddFrame(self.ButtonsFrame, ROOT.TGLayoutHints()) - self.SetWindowName( 'My first GUI' ) - self.MapSubwindows() - self.Resize( self.GetDefaultSize() ) - self.MapWindow() + self.SetWindowName("My first GUI") + self.MapSubwindows() + self.Resize(self.GetDefaultSize()) + self.MapWindow() - def __del__(self): - self.Cleanup() + def __del__(self): + self.Cleanup() -if __name__ == '__main__': - window = pMainFrame( ROOT.gClient.GetRoot(), 200, 200 ) +if __name__ == "__main__": + window = pMainFrame(ROOT.gClient.GetRoot(), 200, 200)