diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/CPyCppyyModule.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/CPyCppyyModule.cxx index 8d12ff5dc6740..53960944b621b 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/CPyCppyyModule.cxx +++ b/bindings/pyroot/cppyy/CPyCppyy/src/CPyCppyyModule.cxx @@ -806,6 +806,60 @@ static PyObject* BindObject(PyObject*, PyObject* args, PyObject* kwds) return BindCppObjectNoCast(addr, cast_type); } +//---------------------------------------------------------------------------- +static PyObject* ValueFromMemory(PyObject*, PyObject* args, PyObject* kwds) +{ +// Build a Python object from the memory at the given address, using the +// Converter for the given type name. See cppyy.ll.value_from_memory for the +// user-facing documentation. + static const char* kwlist[] = {(char*)"type_name", (char*)"address", (char*)"dims", nullptr}; + + const char* type_name = nullptr; + PyObject* pyaddress = nullptr; + PyObject* pydims = nullptr; + if (!PyArg_ParseTupleAndKeywords(args, kwds, "sO|O:value_from_memory", + (char**)kwlist, &type_name, &pyaddress, &pydims)) + return nullptr; + + void* address = PyLong_AsVoidPtr(pyaddress); + if (PyErr_Occurred()) + return nullptr; + + std::vector dims; + if (pydims && pydims != Py_None) { + PyObject* seq = PySequence_Fast(pydims, "dims must be a sequence of integers"); + if (!seq) + return nullptr; + Py_ssize_t ndim = PySequence_Fast_GET_SIZE(seq); + dims.reserve(ndim); + for (Py_ssize_t i = 0; i < ndim; ++i) { + dim_t d = (dim_t)PyLong_AsSsize_t(PySequence_Fast_GET_ITEM(seq, i)); + if (d == (dim_t)-1 && PyErr_Occurred()) { + Py_DECREF(seq); + return nullptr; + } + dims.push_back(d); + } + Py_DECREF(seq); + } + + Converter* cnv = CreateConverter(type_name, Dimensions((dim_t)dims.size(), dims.data())); + if (!cnv) { + PyErr_Format(PyExc_TypeError, "no converter available for type \'%s\'", type_name); + return nullptr; + } + +// an array converter reads through the data pointer, so it wants the address +// of that pointer; a scalar converter wants the address of the value itself + PyObject* result = dims.empty() ? cnv->FromMemory(address) : cnv->FromMemory(&address); + DestroyConverter(cnv); + + if (!result && !PyErr_Occurred()) + PyErr_Format(PyExc_TypeError, "failed to convert a value of type \'%s\' from memory", type_name); + + return result; +} + //---------------------------------------------------------------------------- static PyObject* Move(PyObject*, PyObject* pyobject) { @@ -986,6 +1040,8 @@ static PyMethodDef gCPyCppyyMethods[] = { METH_O, (char*)"Represent an array of objects as raw memory."}, {(char*)"bind_object", (PyCFunction)BindObject, METH_VARARGS | METH_KEYWORDS, (char*) "Create an object of given type, from given address."}, + {(char*)"value_from_memory", (PyCFunction)ValueFromMemory, + METH_VARARGS | METH_KEYWORDS, (char*) "Read a value of given type, from given address."}, {(char*) "move", (PyCFunction)Move, METH_O, (char*)"Cast the C++ object to become movable."}, {(char*) "add_pythonization", (PyCFunction)AddPythonization, diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/LowLevelViews.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/LowLevelViews.cxx index 8e302f74154b8..132b223f079ad 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/LowLevelViews.cxx +++ b/bindings/pyroot/cppyy/CPyCppyy/src/LowLevelViews.cxx @@ -7,6 +7,7 @@ // Standard #include +#include #include #include #include @@ -765,28 +766,89 @@ static PyObject* ll_reshape(CPyCppyy::LowLevelView* self, PyObject* shape) Py_buffer& view = self->fBufInfo; -// verify size match - Py_ssize_t oldsz = 0; +// verify size match (the number of elements is the product of the dimensions) + Py_ssize_t oldsz = 1; for (Py_ssize_t idim = 0; idim < view.ndim; ++idim) { Py_ssize_t nlen = view.shape[idim]; if (nlen == CPyCppyy::UNKNOWN_SIZE || nlen == INT_MAX/view.itemsize /* fake 'max' */) { oldsz = -1; // meaning, unable to check size match break; } - oldsz += view.shape[idim]; + oldsz *= view.shape[idim]; } - if (0 < oldsz) { - Py_ssize_t newsz = 0; - for (Py_ssize_t idim = 0; idim < PyTuple_GET_SIZE(shape); ++idim) - newsz += PyInt_AsSsize_t(PyTuple_GET_ITEM(shape, idim)); - if (oldsz != newsz) { - PyObject* tas = PyObject_Str(shape); - PyErr_Format(PyExc_ValueError, - "cannot reshape array of size %ld into shape %s", (long)oldsz, CPyCppyy_PyText_AsString(tas)); - Py_DECREF(tas); + Py_ssize_t newsz = 1; + for (Py_ssize_t idim = 0; idim < PyTuple_GET_SIZE(shape); ++idim) { + Py_ssize_t nlen = PyInt_AsSsize_t(PyTuple_GET_ITEM(shape, idim)); + if (nlen == -1 && PyErr_Occurred()) + return nullptr; + newsz *= nlen; + } + + if (0 < oldsz && oldsz != newsz) { + PyObject* tas = PyObject_Str(shape); + PyErr_Format(PyExc_ValueError, + "cannot reshape array of size %ld into shape %s", (long)oldsz, CPyCppyy_PyText_AsString(tas)); + Py_DECREF(tas); + return nullptr; + } + +// A multi-dimensional view is not simply a Py_buffer with more entries in its +// shape: element access projects sub-views through a dedicated converter that +// is chosen when the view is created. Adding dimensions to a one-dimensional +// view therefore requires re-creating it through the same creator that made +// it, rather than patching up the Py_buffer in place. +// +// Only rank-1 views can be grown this way: their data is by construction a +// flat block. A view that is already multi-dimensional carries a layout that +// the shape alone does not describe (T** data, for example, is an array of +// row pointers, not a contiguous block), so for those the dimensions are +// merely filled in below, leaving the layout as it was. + if (view.ndim == 1 && 1 < PyTuple_GET_SIZE(shape)) { + if (!self->fCreator) { + PyErr_SetString(PyExc_TypeError, + "this low level view does not support multi-dimensional reshaping"); return nullptr; } + + std::vector dims; + dims.reserve(PyTuple_GET_SIZE(shape)); + for (Py_ssize_t idim = 0; idim < PyTuple_GET_SIZE(shape); ++idim) + dims.push_back((CPyCppyy::dim_t)PyInt_AsSsize_t(PyTuple_GET_ITEM(shape, idim))); + + // the creator takes the address the view was originally created from: the + // address of the data pointer if there is one, the data itself otherwise + CPyCppyy::LowLevelView* llnew = self->fCreator( + self->fBuf ? (void*)self->fBuf : view.buf, + CPyCppyy::Dimensions((CPyCppyy::dim_t)dims.size(), dims.data())); + if (!llnew) + return nullptr; + + // ownership of the data (if any) stays with self, so keep that flag and + // make sure llnew does not free anything that is now ours + intptr_t owner = (intptr_t)view.internal & CPyCppyy::LowLevelView::kIsOwner; + + PyMem_Free(view.shape); + PyMem_Free(view.strides); + if (self->fElemCnv != self->fConverter && self->fElemCnv && self->fElemCnv->HasState()) + delete self->fElemCnv; + if (self->fConverter && self->fConverter->HasState()) + delete self->fConverter; + + self->fBufInfo = llnew->fBufInfo; + self->fBuf = llnew->fBuf; + self->fConverter = llnew->fConverter; + self->fElemCnv = llnew->fElemCnv; + (intptr_t&)self->fBufInfo.internal |= owner; + + llnew->fBufInfo.shape = nullptr; + llnew->fBufInfo.strides = nullptr; + (intptr_t&)llnew->fBufInfo.internal &= ~CPyCppyy::LowLevelView::kIsOwner; + llnew->fConverter = nullptr; + llnew->fElemCnv = nullptr; + Py_DECREF((PyObject*)llnew); + + Py_RETURN_NONE; } // reshape @@ -812,6 +874,14 @@ static PyObject* ll_reshape(CPyCppyy::LowLevelView* self, PyObject* shape) set_strides(view, itemsize, false /* by definition not fixed */); +// a rank-1 view hands out elements rather than sub-views, so drop any +// projecting converter that a previous, higher-rank shape installed + if (view.ndim == 1 && self->fConverter != self->fElemCnv) { + if (self->fConverter && self->fConverter->HasState()) + delete self->fConverter; + self->fConverter = self->fElemCnv; + } + Py_RETURN_NONE; } diff --git a/bindings/pyroot/cppyy/cppyy/python/cppyy/ll.py b/bindings/pyroot/cppyy/cppyy/python/cppyy/ll.py index 9fb6034ce0843..25e4dc340d792 100644 --- a/bindings/pyroot/cppyy/cppyy/python/cppyy/ll.py +++ b/bindings/pyroot/cppyy/cppyy/python/cppyy/ll.py @@ -24,6 +24,7 @@ 'free', 'array_new', 'array_delete', + 'value_from_memory', 'signals_as_exception', 'set_signals_as_exception', 'FatalError', @@ -44,6 +45,34 @@ def argc(): """Return C's argc for use with cppyy/ctypes.""" return len(sys.argv) +def value_from_memory(type_name, address, dims=None): + """Read a value of the C++ type `type_name` from `address`. + + This is the reading half of the conversion that cppyy performs when a + function returns, or a data member is read: given a type name and a raw + address, it hands back the Python object that cppyy would have produced. + It is meant for code that has an address and a type name in hand, but no + C++ entity to read them from, such as a framework exposing its own data + description. + + `address` is an integer, as returned by `addressof`. `type_name` is any + C++ type name that cppyy can resolve, including typedefs. For a class + type a bound proxy is returned, for a builtin type a Python value. + + If `dims` is given, it is a sequence of integers describing the shape of + an array, `address` is taken to be the start of the array data, and a + `LowLevelView` of that shape is returned. The type name should then name + the element type followed by `[]`, e.g. `"double[]"`. Pass a single-entry + sequence for a one-dimensional array. + + v = ll.value_from_memory('double', addr) # a float + a = ll.value_from_memory('double[]', addr, (2, 3)) # a 2x3 view + + Note that no lifetime or bounds checking is or can be done: the caller + vouches for the address, the type and the shape. + """ + return cppyy._backend.value_from_memory(type_name, address, dims) + # import low-level python converters for _name in ['addressof', 'as_cobject', 'as_capsule', 'as_ctypes', 'as_memoryview']: try: diff --git a/bindings/pyroot/pythonizations/CMakeLists.txt b/bindings/pyroot/pythonizations/CMakeLists.txt index 04aeb31ac581a..9d69c50847e0b 100644 --- a/bindings/pyroot/pythonizations/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/CMakeLists.txt @@ -13,7 +13,6 @@ set(cpp_sources src/RPyROOTApplication.cxx src/GenericPyz.cxx src/TClassPyz.cxx - src/TTreePyz.cxx src/CPPInstancePyz.cxx ) diff --git a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_ttree.py b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_ttree.py index 77cd655577c3a..179bd506f6a31 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_ttree.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_ttree.py @@ -159,8 +159,6 @@ \endpythondoc """ -from ROOT.libROOTPythonizations import BranchPyz, GetBranchAttr - from . import pythonization from ._memory_utils import ( _constructor_releasing_ownership, @@ -170,6 +168,103 @@ from ._rvec import _get_cpp_type_from_numpy_type +_branch_lookups = None +_branch_ptr_to_ptr = None + + +class _BranchLookups(object): + """The entities that reading a branch needs, looked up once. + + Every name resolved through the ROOT module goes through the facade's + __getattr__, which is far too expensive to repeat per branch access: it + dominated the cost of `tree.branch` when these were looked up inline. + None of them can change over the lifetime of a session, so they are + resolved on first use and kept. + """ + + __slots__ = ("helpers", "branch_element", "branch_object", "leaf_element", "leaf_object", "instance", "ll") + + +def _lookups(): + """Return the cached branch lookups, doing the one-off setup if needed. + + TBranch::GetAddress() and TBranchElement::GetObject() return char*, which + cppyy faithfully turns into a Python str, throwing the pointer value away. + Neither class offers a void* accessor and fAddress is protected, so the + addresses are only reachable through wrappers with a different return type. + These are declared on first use rather than at import, so that sessions + that never touch a branch do not pay for compiling them. + """ + global _branch_lookups + + if _branch_lookups is None: + import ROOT + from cppyy import ll + + ROOT.gInterpreter.Declare(""" + namespace ROOT::Internal::PyROOT { + inline intptr_t GetBranchAddress(TBranch *branch) + { + return reinterpret_cast(branch->GetAddress()); + } + inline intptr_t GetBranchAddressDeref(TBranch *branch) + { + char *address = branch->GetAddress(); + return address ? reinterpret_cast(*reinterpret_cast(address)) : 0; + } + inline intptr_t GetBranchElementObject(TBranchElement *branch) + { + return reinterpret_cast(branch->GetObject()); + } + } + """) + + lookups = _BranchLookups() + lookups.helpers = ROOT.Internal.PyROOT + lookups.branch_element = ROOT.TBranchElement.Class() + lookups.branch_object = ROOT.TBranchObject.Class() + lookups.leaf_element = ROOT.TLeafElement.Class() + lookups.leaf_object = ROOT.TLeafObject.Class() + lookups.instance = ROOT._cppyy.types.Instance + lookups.ll = ll + _branch_lookups = lookups + + return _branch_lookups + + +def _ptr_to_ptr_brancher(): + """Return the C++ helper that branches on the address of a pointer. + + The T** overloads of TTree::Branch want the address of the pointer to the + object. Where that address lives depends on the kind of proxy holding it: a + proxy for an object keeps the object pointer itself, a proxy for a reference + to a pointer keeps the address of the caller's pointer. Deriving it here + would mean restating that rule, so instead the helper takes a T** and lets + cppyy apply the rule, as it does for any other C++ function taking one. + + Declared on first use rather than at import, so that sessions that never + branch on an object do not pay for compiling it. + """ + global _branch_ptr_to_ptr + + if _branch_ptr_to_ptr is None: + import ROOT + + ROOT.gInterpreter.Declare(""" + namespace ROOT::Internal::PyROOT { + template + TBranch *BranchPtrToPtr(TTree &tree, const char *name, const char *className, T **obj, + Int_t bufsize = 32000, Int_t splitlevel = 99) + { + return tree.Branch(name, className, reinterpret_cast(obj), bufsize, splitlevel); + } + } + """) + _branch_ptr_to_ptr = ROOT.Internal.PyROOT.BranchPtrToPtr + + return _branch_ptr_to_ptr + + # TTree iterator def _TTree__iter__(self): i = 0 @@ -305,18 +400,215 @@ def _SetBranchAddress(self, bname, addr, *args, **kwargs): return self._OriginalSetBranchAddress(bname, addr, ptr=tbranch_ptr, realClass=cl, datatype=tp, isptr=False) +def _get_address_of(obj): + """Return the address of the buffer or proxied object `obj` points at. + + Returns None for anything that does not carry an address, and deliberately + also for text-like objects, which do but are never meant as a branch buffer. + """ + import ROOT + + if isinstance(obj, ROOT._cppyy.types.Instance): + return ROOT._cppyy.addressof(instance=obj, byref=False) + + if isinstance(obj, (str, bytes)): + return None + + try: + return ROOT._cppyy.ll.addressof(obj) + except TypeError: + return None + + +def _try_branch_leaf_list_overload(self, args): + """Try to match TTree::Branch(const char*, void*, const char*, Int_t = 32000).""" + if not (3 <= len(args) <= 4): + return None + name, address, leaflist = args[0], args[1], args[2] + if not isinstance(name, str) or not isinstance(leaflist, str): + return None + if len(args) == 4 and not isinstance(args[3], int): + return None + + import ctypes + + buf = _get_address_of(address) + if not buf: + return None + + return self._OriginalBranch(name, ctypes.c_void_p(buf), leaflist, *args[3:]) + + +def _try_branch_ptr_to_ptr_overloads(self, args): + """Try to match one of the TTree::Branch overloads taking a T**: + + - ( const char*, const char*, T**, Int_t = 32000, Int_t = 99 ) + - ( const char*, T**, Int_t = 32000, Int_t = 99 ) + """ + import ROOT + + if len(args) < 2 or not isinstance(args[0], str): + return None + + name = args[0] + if isinstance(args[1], str): + # the class name is given explicitly + class_name, address, rest = args[1], args[2] if len(args) > 2 else None, args[3:] + else: + class_name, address, rest = None, args[1], args[2:] + + if address is None or any(not isinstance(arg, int) for arg in rest) or len(rest) > 2: + return None + + if isinstance(address, ROOT._cppyy.types.Instance): + # Hand the proxy to a helper taking a T** and let cppyy work out which + # address that is. T is the proxied type rather than class_name, which + # the caller is free to give as a base of it, or as an equivalent + # spelling that is not the one cppyy knows the proxy by. + proxy_type = type(address).__cpp_name__ + return _ptr_to_ptr_brancher()[proxy_type](self, name, class_name or proxy_type, address, *rest) + + buf = _get_address_of(address) + if not buf or not class_name: + return None + + import ctypes + + return self._OriginalBranch(name, class_name, ctypes.c_void_p(buf), *rest) + + def _Branch(self, *args): - # Modify the behaviour if args is one of: - # ( const char*, void*, const char*, Int_t = 32000 ) - # ( const char*, const char*, T**, Int_t = 32000, Int_t = 99 ) - # ( const char*, T**, Int_t = 32000, Int_t = 99 ) - res = BranchPyz(self, *args) + """ + Pythonization for TTree::Branch. + + Modify the behaviour of Branch so that proxy references can be passed as + arguments from the Python side, more precisely in cases where the C++ + implementation of the method expects the address of a pointer. + + For example: + ``` + v = ROOT.std.vector('int')() + t.Branch('my_vector_branch', v) + ``` + + The following signatures are treated in this pythonization: + - ( const char*, void*, const char*, Int_t = 32000 ) + - ( const char*, const char*, T**, Int_t = 32000, Int_t = 99 ) + - ( const char*, T**, Int_t = 32000, Int_t = 99 ) + """ + if len(args) >= 2: + res = _try_branch_leaf_list_overload(self, args) + if res is not None: + return res + + res = _try_branch_ptr_to_ptr_overloads(self, args) + if res is not None: + return res + + # Fall back to the original implementation for the rest of overloads + return self._OriginalBranch(*args) + + +def _search_for_branch(tree, name): + branch = tree.GetBranch(name) + if not branch: + # for benefit of naming of sub-branches, the actual name may have a + # trailing '.' + branch = tree.GetBranch(name + ".") + return branch + + +def _has_single_leaf(branch): + leaves = branch.GetListOfLeaves() + # i.e. if unambiguously only this one + return bool(leaves.GetSize()) and leaves.First() == leaves.Last() + + +def _search_for_leaf(tree, name, branch): + leaf = tree.GetLeaf(name) + if branch and not leaf: + leaf = branch.GetLeaf(name) + if not leaf and _has_single_leaf(branch): + leaf = branch.GetListOfLeaves().At(0) + return leaf + + +def _resolve_branch(tree, name, branch): + """Return the address and type name of the object held by a branch. + + Returns (None, "") if the branch does not hold an object, in which case the + caller should look for a leaf instead. An address of 0 with a non-empty type + name means failure, and is reported to the user as a typed null object. + """ + lookups = _lookups() + helpers = lookups.helpers + + # for partial return of a split object + if branch.InheritsFrom(lookups.branch_element): + current_class = branch.GetCurrentClass() + if current_class and current_class != branch.GetTargetClass() and branch.GetID() >= 0: + offset = branch.GetInfo().GetElements().At(branch.GetID()).GetOffset() + return helpers.GetBranchElementObject(branch) + offset, current_class.GetName() + + # for return of a full object + if branch.IsA() in (lookups.branch_element, lookups.branch_object): + if helpers.GetBranchAddress(branch): + return helpers.GetBranchAddressDeref(branch), branch.GetClassName() + + # try leaf, otherwise indicate failure by returning a typed null object + if not tree.GetLeaf(name) and not _has_single_leaf(branch): + return 0, branch.GetClassName() + + return None, "" + + +def _get_multi_dims(title): + """Extract the static dimensions from the title of a TLeaf. + + The title of a multi-dimensional leaf carries its dimensions as + `name[dim1][dim2]...`. In the current implementation of TLeaf there is no + way to get at them other than by parsing that string. + """ + import re + + return [int(dim) for dim in re.findall(r"\[([^\]]*)\]", title) if dim] - if res is None: - # Fall back to the original implementation for the rest of overloads - res = self._OriginalBranch(*args) - return res +def _wrap_leaf(leaf): + """Read the value of a leaf for the entry the tree is currently on.""" + lookups = _lookups() + ll = lookups.ll + + if leaf.GetLenStatic() > 1 or leaf.GetLeafCount(): + # array types + is_static = leaf.GetLenStatic() > 1 + type_name = leaf.GetTypeName() + + dims = [leaf.GetNdata()] + title = leaf.GetTitle() + if title.count("[") >= 2: + # multidimensional array case + dims = _get_multi_dims(title) + + address = 0 + branch = leaf.GetBranch() + if branch: + address = lookups.helpers.GetBranchAddress(branch) + if not address: + address = ll.addressof(leaf.GetValuePointer()) + + return ll.value_from_memory(type_name + ("[]" if is_static else "*"), address, dims) + + value_pointer = leaf.GetValuePointer() + if value_pointer: + # value types + address = ll.addressof(value_pointer) + if leaf.IsA() in (lookups.leaf_element, lookups.leaf_object): + # the leaf holds a pointer to the value, rather than the value + address = ll.value_from_memory("intptr_t", address) + return ll.value_from_memory(leaf.GetTypeName(), address) + + return None def _TTree__getattr__(self, key): @@ -326,21 +618,35 @@ def _TTree__getattr__(self, key): Allow access to branches/leaves as if they were Python data attributes of the tree (e.g. mytree.branch). - To avoid using the CPyCppyy API, any necessary cast is done here on the - Python side. The GetBranchAttr() function encodes a necessary cast in the - second element of the output tuple, which is a string with the required - type name. - Parameters: self (TTree): The instance of the TTree object from which the attribute is being retrieved. key (str): The name of the branch to retrieve from the TTree object. """ - import ROOT - - out, cast_type = GetBranchAttr(self, key) - if cast_type: - out = ROOT._cppyy.ll.cast[cast_type](out) - return out + ll = _lookups().ll + + # deal with possible aliasing + name = self.GetAlias(key) or key + + # search for branch first (typical for objects) + branch = _search_for_branch(self, name) + + if branch: + # found a branched object, wrap its address for the object it represents + address, type_name = _resolve_branch(self, name, branch) + if type_name: + return ll.cast[type_name + "*"](address) + + # if not, try leaf + leaf = _search_for_leaf(self, name, branch) + if leaf: + # found a leaf, extract value and wrap with a Python object + # according to its type + value = _wrap_leaf(leaf) + if value is not None: + return value + + # confused + raise AttributeError("'{}' object has no attribute '{}'".format(self.IsA().GetName(), name)) def _TTree_CloneTree(self, *args, **kwargs): diff --git a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx index d6ba1aa297b6d..5ec2f527008eb 100644 --- a/bindings/pyroot/pythonizations/src/PyROOTModule.cxx +++ b/bindings/pyroot/pythonizations/src/PyROOTModule.cxx @@ -183,12 +183,8 @@ void GetBuffer(PyObject *pyobject, void *&buf) static PyMethodDef gPyROOTMethods[] = { {"AddCPPInstancePickling", (PyCFunction)PyROOT::AddCPPInstancePickling, METH_NOARGS, "Add a custom pickling mechanism for Cppyy Python proxy objects"}, - {"GetBranchAttr", (PyCFunction)PyROOT::GetBranchAttr, METH_VARARGS, - "Allow to access branches as tree attributes"}, {"AddTClassDynamicCastPyz", (PyCFunction)PyROOT::AddTClassDynamicCastPyz, METH_VARARGS, "Cast the void* returned by TClass::DynamicCast to the right type"}, - {"BranchPyz", (PyCFunction)PyROOT::BranchPyz, METH_VARARGS, - "Fully enable the use of TTree::Branch from Python"}, {"AddPrettyPrintingPyz", (PyCFunction)PyROOT::AddPrettyPrintingPyz, METH_VARARGS, "Add pretty printing pythonization"}, {"InitApplication", (PyCFunction)PyROOT::RPyROOTApplication::InitApplication, METH_VARARGS, diff --git a/bindings/pyroot/pythonizations/src/PyROOTPythonize.h b/bindings/pyroot/pythonizations/src/PyROOTPythonize.h index aec7ed708b3f9..7246bf2970062 100644 --- a/bindings/pyroot/pythonizations/src/PyROOTPythonize.h +++ b/bindings/pyroot/pythonizations/src/PyROOTPythonize.h @@ -20,8 +20,6 @@ PyObject *AddCPPInstancePickling(PyObject *self, PyObject *args); PyObject *AddPrettyPrintingPyz(PyObject *self, PyObject *args); -PyObject *GetBranchAttr(PyObject *self, PyObject *args); -PyObject *BranchPyz(PyObject *self, PyObject *args); PyObject *AddTClassDynamicCastPyz(PyObject *self, PyObject *args); diff --git a/bindings/pyroot/pythonizations/src/TTreePyz.cxx b/bindings/pyroot/pythonizations/src/TTreePyz.cxx deleted file mode 100644 index 480e8a55d19e3..0000000000000 --- a/bindings/pyroot/pythonizations/src/TTreePyz.cxx +++ /dev/null @@ -1,409 +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. * - *************************************************************************/ - -// Bindings -#include - -// TODO: refactor public CPyCppyy API such that this forward declaration is not -// needed anymore. Including "CPyCppyy/API.h" should be enough. -namespace CPyCppyy { -typedef Py_ssize_t dim_t; -} // namespace CPyCppyy - -#include "../../cppyy/CPyCppyy/src/Cppyy.h" -#include "../../cppyy/CPyCppyy/src/CPPInstance.h" -#include "../../cppyy/CPyCppyy/src/ProxyWrappers.h" -#include "../../cppyy/CPyCppyy/src/Dimensions.h" - -#include "CPyCppyy/API.h" - -#include "PyROOTPythonize.h" - -// ROOT -#include "TClass.h" -#include "TTree.h" -#include "TBranch.h" -#include "TBranchElement.h" -#include "TBranchObject.h" -#include "TLeaf.h" -#include "TLeafElement.h" -#include "TLeafObject.h" -#include "TStreamerElement.h" -#include "TStreamerInfo.h" - -#include -#include - -namespace { - -// Get the TClass of the C++ object proxied by pyobj -TClass *GetTClass(PyObject *pyobj) -{ - return TClass::GetClass(CPyCppyy::Instance_GetScopedFinalName(pyobj).c_str()); -} - -} // namespace - -using namespace CPyCppyy; - -namespace PyROOT{ -void GetBuffer(PyObject *pyobject, void *&buf); -} - -static TBranch *SearchForBranch(TTree *tree, const char *name) -{ - TBranch *branch = tree->GetBranch(name); - if (!branch) { - // for benefit of naming of sub-branches, the actual name may have a trailing '.' - branch = tree->GetBranch((std::string(name) + '.').c_str()); - } - return branch; -} - -static TLeaf *SearchForLeaf(TTree *tree, const char *name, TBranch *branch) -{ - TLeaf *leaf = tree->GetLeaf(name); - if (branch && !leaf) { - leaf = branch->GetLeaf(name); - if (!leaf) { - TObjArray *leaves = branch->GetListOfLeaves(); - if (leaves->GetSize() && (leaves->First() == leaves->Last())) { - // i.e., if unambiguously only this one - leaf = (TLeaf *)leaves->At(0); - } - } - } - return leaf; -} - -static std::pair ResolveBranch(TTree *tree, const char *name, TBranch *branch) -{ - // for partial return of a split object - if (branch->InheritsFrom(TBranchElement::Class())) { - TBranchElement *be = (TBranchElement *)branch; - if (be->GetCurrentClass() && (be->GetCurrentClass() != be->GetTargetClass()) && (0 <= be->GetID())) { - Long_t offset = ((TStreamerElement *)be->GetInfo()->GetElements()->At(be->GetID()))->GetOffset(); - return {be->GetObject() + offset, be->GetCurrentClass()->GetName()}; - } - } - - // for return of a full object - if (branch->IsA() == TBranchElement::Class() || branch->IsA() == TBranchObject::Class()) { - if (branch->GetAddress()) - return {*(void **)branch->GetAddress(), branch->GetClassName()}; - - // try leaf, otherwise indicate failure by returning a typed null-object - TObjArray *leaves = branch->GetListOfLeaves(); - if (!tree->GetLeaf(name) && !(leaves->GetSize() && (leaves->First() == leaves->Last()))) - return {nullptr, branch->GetClassName()}; - } - - return {nullptr, ""}; -} - -/** - * @brief Extracts static dimensions from the title of a TLeaf object. - * - * The function assumes that the title of the TLeaf object contains dimensions - * in the format `[dim1][dim2]...`. - * - * @note In the current implementation of TLeaf, there is no way to extract the - * dimensions without string parsing. - * - * @param title title of the TLeaf object from which to extract dimensions. - * @return std::vector A vector containing the extracted dimensions. - */ -static std::vector getMultiDims(std::string const &title) -{ - std::vector dims; - std::stringstream ss{title}; - - while (ss.good()) { - std::string substr; - getline(ss, substr, '['); - getline(ss, substr, ']'); - if (!substr.empty()) { - dims.push_back(std::stoi(substr)); - } - } - - return dims; -} - -static PyObject *WrapLeaf(TLeaf *leaf) -{ - if (1 < leaf->GetLenStatic() || leaf->GetLeafCount()) { - bool isStatic = 1 < leaf->GetLenStatic(); - // array types - std::string typeName = leaf->GetTypeName(); - std::vector dimsVec{leaf->GetNdata()}; - std::string title = leaf->GetTitle(); - // Multidimensional array case - if (std::count(title.begin(), title.end(), '[') >= 2) { - dimsVec = getMultiDims(title); - } - CPyCppyy::Dimensions dims{static_cast(dimsVec.size()), dimsVec.data()}; - Converter *pcnv = CreateConverter(typeName + (isStatic ? "[]" : "*"), dims); - - void *address = 0; - if (leaf->GetBranch()) - address = (void *)leaf->GetBranch()->GetAddress(); - if (!address) - address = (void *)leaf->GetValuePointer(); - - PyObject *value = pcnv->FromMemory(&address); - CPyCppyy::DestroyConverter(pcnv); - - return value; - } else if (leaf->GetValuePointer()) { - // value types - Converter *pcnv = CreateConverter(leaf->GetTypeName()); - PyObject *value = 0; - if (leaf->IsA() == TLeafElement::Class() || leaf->IsA() == TLeafObject::Class()) - value = pcnv->FromMemory((void *)*(void **)leaf->GetValuePointer()); - else - value = pcnv->FromMemory((void *)leaf->GetValuePointer()); - CPyCppyy::DestroyConverter(pcnv); - - return value; - } - - return nullptr; -} - -// Allow access to branches/leaves as if they were data members Returns a -// Python tuple where the first element is either the desired CPyCppyy proxy, -// or an address that still needs to be wrapped by the caller in a proxy using -// cppyy.ll.cast. In the latter case, the second tuple element is the target -// type name. Otherwise, the second element is an empty string. -PyObject *PyROOT::GetBranchAttr(PyObject * /*self*/, PyObject *args) -{ - PyObject *self = nullptr; - PyObject *pyname = nullptr; - - PyArg_ParseTuple(args, "OU:GetBranchAttr", &self, &pyname); - - const char *name_possibly_alias = PyUnicode_AsUTF8AndSize(pyname, nullptr); - if (!name_possibly_alias) - return nullptr; - - // get hold of actual tree - auto tree = (TTree *)GetTClass(self)->DynamicCast(TTree::Class(), CPyCppyy::Instance_AsVoidPtr(self)); - - if (!tree) { - PyErr_SetString(PyExc_ReferenceError, "attempt to access a null-pointer"); - return 0; - } - - // deal with possible aliasing - const char *name = tree->GetAlias(name_possibly_alias); - if (!name) - name = name_possibly_alias; - - // search for branch first (typical for objects) - TBranch *branch = SearchForBranch(tree, name); - - if (branch) { - // found a branched object, wrap its address for the object it represents - const auto [finalAddressVoidPtr, finalTypeName] = ResolveBranch(tree, name, branch); - if (!finalTypeName.empty()) { - PyObject *outTuple = PyTuple_New(2); - PyTuple_SetItem(outTuple, 0, PyLong_FromLongLong((intptr_t)finalAddressVoidPtr)); - PyTuple_SetItem(outTuple, 1, PyUnicode_FromString((finalTypeName + "*").c_str())); - return outTuple; - } - } - - // if not, try leaf - if (TLeaf *leaf = SearchForLeaf(tree, name, branch)) { - // found a leaf, extract value and wrap with a Python object according to its type - auto wrapper = WrapLeaf(leaf); - if (wrapper != nullptr) { - PyObject *outTuple = PyTuple_New(2); - PyTuple_SetItem(outTuple, 0, wrapper); - PyTuple_SetItem(outTuple, 1, PyUnicode_FromString("")); - return outTuple; - } - } - - // confused - PyErr_Format(PyExc_AttributeError, "\'%s\' object has no attribute \'%s\'", tree->IsA()->GetName(), name); - return 0; -} - -//////////////////////////////////////////////////////////////////////////// -/// Try to match the arguments of TTree::Branch to the following overload: -/// - ( const char*, void*, const char*, Int_t = 32000 ) -/// If the match succeeds, invoke Branch on the C++ tree with the right -/// arguments. -PyObject *TryBranchLeafListOverload(int argc, PyObject *args) -{ - PyObject *treeObj = nullptr; - PyObject *name = nullptr, *address = nullptr, *leaflist = nullptr, *bufsize = nullptr; - - if (PyArg_ParseTuple(args, "OO!OO!|O!:Branch", &treeObj, &PyUnicode_Type, &name, &address, &PyUnicode_Type, - &leaflist, &PyLong_Type, &bufsize)) { - - auto tree = (TTree *)GetTClass(treeObj)->DynamicCast(TTree::Class(), CPyCppyy::Instance_AsVoidPtr(treeObj)); - if (!tree) { - PyErr_SetString(PyExc_TypeError, "TTree::Branch must be called with a TTree instance as first argument"); - return nullptr; - } - - void *buf = nullptr; - if (CPyCppyy::Instance_Check(address)) - buf = CPyCppyy::Instance_AsVoidPtr(address); - else - PyROOT::GetBuffer(address, buf); - - if (buf) { - TBranch *branch = nullptr; - const char *nameString = PyUnicode_AsUTF8AndSize(name, nullptr); - if (!nameString) { - return nullptr; - } - const char *leaflistString = PyUnicode_AsUTF8AndSize(leaflist, nullptr); - if (!leaflistString) { - return nullptr; - } - if (argc == 5) { - branch = tree->Branch(nameString, buf, leaflistString, PyLong_AsLong(bufsize)); - } else { - branch = tree->Branch(nameString, buf, leaflistString); - } - - return BindCppObject(branch, Cppyy::GetScope("TBranch")); - } - } - PyErr_Clear(); - - Py_RETURN_NONE; -} - -//////////////////////////////////////////////////////////////////////////// -/// Try to match the arguments of TTree::Branch to one of the following -/// overloads: -/// - ( const char*, const char*, T**, Int_t = 32000, Int_t = 99 ) -/// - ( const char*, T**, Int_t = 32000, Int_t = 99 ) -/// If the match succeeds, invoke Branch on the C++ tree with the right -/// arguments. -PyObject *TryBranchPtrToPtrOverloads(int argc, PyObject *args) -{ - PyObject *treeObj = nullptr; - PyObject *name = nullptr, *clName = nullptr, *address = nullptr, *bufsize = nullptr, *splitlevel = nullptr; - - auto bIsMatch = false; - if (PyArg_ParseTuple(args, "OO!O!O|O!O!:Branch", &treeObj, &PyUnicode_Type, &name, &PyUnicode_Type, &clName, - &address, &PyLong_Type, &bufsize, &PyLong_Type, &splitlevel)) { - bIsMatch = true; - } else { - PyErr_Clear(); - if (PyArg_ParseTuple(args, "OO!O|O!O!", &treeObj, &PyUnicode_Type, &name, &address, &PyLong_Type, &bufsize, - &PyLong_Type, &splitlevel)) { - bIsMatch = true; - } else { - PyErr_Clear(); - } - } - - if (bIsMatch) { - auto tree = (TTree *)GetTClass(treeObj)->DynamicCast(TTree::Class(), CPyCppyy::Instance_AsVoidPtr(treeObj)); - if (!tree) { - PyErr_SetString(PyExc_TypeError, "TTree::Branch must be called with a TTree instance as first argument"); - return nullptr; - } - - std::string klName; - if (clName) { - const char *clNameString = PyUnicode_AsUTF8AndSize(clName, nullptr); - if (!clNameString) { - return nullptr; - } - klName = clNameString; - } - void *buf = nullptr; - - if (CPyCppyy::Instance_Check(address)) { - if (((CPPInstance *)address)->fFlags & CPPInstance::kIsReference) - buf = (void *)((CPPInstance *)address)->fObject; - else - buf = (void *)&((CPPInstance *)address)->fObject; - - if (!clName) { - klName = GetTClass(address)->GetName(); - argc += 1; - } - } else { - PyROOT::GetBuffer(address, buf); - } - - if (buf && !klName.empty()) { - TBranch *branch = nullptr; - const char *nameString = nullptr; - if (argc == 4 || argc == 5 || argc == 6) { - nameString = PyUnicode_AsUTF8AndSize(name, nullptr); - if (!nameString) { - return nullptr; - } - } - if (argc == 4) { - branch = tree->Branch(nameString, klName.c_str(), buf); - } else if (argc == 5) { - branch = tree->Branch(nameString, klName.c_str(), buf, PyLong_AsLong(bufsize)); - } else if (argc == 6) { - branch = tree->Branch(nameString, klName.c_str(), buf, PyLong_AsLong(bufsize), - PyLong_AsLong(splitlevel)); - } - - return BindCppObject(branch, Cppyy::GetScope("TBranch")); - } - } - - Py_RETURN_NONE; -} - -//////////////////////////////////////////////////////////////////////////// -/// \brief Add pythonization for TTree::Branch. -/// \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. -/// -/// Modify the behaviour of Branch so that proxy references can be passed -/// as arguments from the Python side, more precisely in cases where the C++ -/// implementation of the method expects the address of a pointer. -/// -/// For example: -/// ~~~{.py} -/// v = ROOT.std.vector('int')() -/// t.Branch('my_vector_branch', v) -/// ~~~ -/// -/// The following signatures are treated in this pythonization: -/// - ( const char*, void*, const char*, Int_t = 32000 ) -/// - ( const char*, const char*, T**, Int_t = 32000, Int_t = 99 ) -/// - ( const char*, T**, Int_t = 32000, Int_t = 99 ) -PyObject *PyROOT::BranchPyz(PyObject * /* self */, PyObject *args) -{ - int argc = PyTuple_Size(args); - - if (argc >= 3) { // We count the TTree proxy object too - auto branch = TryBranchLeafListOverload(argc, args); - if (branch != Py_None) - return branch; - - branch = TryBranchPtrToPtrOverloads(argc, args); - if (branch != Py_None) - return branch; - } - - // Not the overload we wanted to pythonize, return None - Py_RETURN_NONE; -} diff --git a/bindings/pyroot/pythonizations/test/ttree_branch.py b/bindings/pyroot/pythonizations/test/ttree_branch.py index c078322b50a29..56009aab98f96 100644 --- a/bindings/pyroot/pythonizations/test/ttree_branch.py +++ b/bindings/pyroot/pythonizations/test/ttree_branch.py @@ -30,6 +30,23 @@ def setUpClass(cls): }; """) + # Declared separately: TreeHelper.h, which ttree.py uses, declares an + # identical MyStruct, so the block above is rejected whole when the two + # test files share an interpreter. Keep what is only needed here out of it. + ROOT.gInterpreter.Declare(""" + #include + #include + + // Reading a pointer data member gives a proxy for a reference to a + // pointer, which is bound differently from a proxy for an object. + struct MyHolder { + std::vector *myvec = new std::vector(); + }; + + intptr_t AddressOfMyVec(MyHolder *h) { return reinterpret_cast(&h->myvec); } + intptr_t AddressOfBranch(TBranch *b) { return reinterpret_cast(b->GetAddress()); } + """) + # Helpers def create_file_and_tree(self): f = ROOT.TFile(self.filename, 'RECREATE') @@ -169,7 +186,42 @@ def test12_read_vector_branch(self): for elem in v: self.assertEqual(elem, self.fval) - def test13_write_fallback_case(self): + def test13_write_reference_proxy_branch(self): + # A proxy for a reference to a pointer, such as the one obtained by + # reading a pointer data member, holds the address of that pointer, + # while a proxy for an object holds the object itself. Branch needs the + # former in both cases; taking the latter binds the branch to the + # proxy's own memory, so that filling writes nothing and the proxy, + # a temporary here, is gone by the time the tree is filled. + f,t = self.create_file_and_tree() + + h = ROOT.MyHolder() + h.myvec.assign(self.arraysize, self.fval) + + # Assert on the address before filling: filling through a branch bound + # to a dead proxy is undefined, and would take the test down with it + for i, args in enumerate([('refvectorb0', h.myvec), + ('refvectorb1', h.myvec, 32000), + ('refvectorb2', h.myvec, 32000, 99), + ('refvectorb3', 'std::vector', h.myvec), + ('refvectorb4', 'std::vector', h.myvec, 32000), + ('refvectorb5', 'std::vector', h.myvec, 32000, 99)]): + b = t.Branch(*args) + self.assertEqual(ROOT.AddressOfBranch(b), ROOT.AddressOfMyVec(h), + 'branch {} not bound to &MyHolder::myvec'.format(i)) + + self.fill_and_close(f, t) + + def test14_read_reference_proxy_branch(self): + f,t = self.get_tree() + + for entry in t: + for v in [ getattr(entry, 'refvectorb' + str(i)) for i in range(6) ]: + self.assertEqual(len(v), self.arraysize) + for elem in v: + self.assertEqual(elem, self.fval) + + def test15_write_fallback_case(self): f,t = self.create_file_and_tree() # Test an overload that uses the original Branch proxy @@ -182,7 +234,7 @@ def test13_write_fallback_case(self): self.fill_and_close(f, t) - def test14_read_fallback_case(self): + def test16_read_fallback_case(self): f,t = self.get_tree() for entry in t: