Skip to content

Commit

Permalink
[PyROOT] Remove TClonesArray.__setitem__ pythonization
Browse files Browse the repository at this point in the history
The `TClonesArray.__setitem__` pythonization was introduced in commit
67db9e5 with PR #3423.

However, this pythonization is quite unnatural, because a `TObjArray`
should not adopt external memory. In fact, on in the C++ side, all
`TObjArray`-inherited methods that take a `TObject *` and that would
adopt these objects are flagged as `MayNotUse` for good reasons!

Supporting this element setting only on the Python side therefore comes
with a very high cost of hacking deep into CPyCppyy, and the benefit of
this is not clear because the PR that introduced this feature didn't
refer to any user request.

Hence, this commit suggests to remove this pythonization, with the
ultimate goal of relying less on the usage of CPyCppyy internals.
  • Loading branch information
guitargeek committed Jan 30, 2024
1 parent 65afe27 commit 0afffc6
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 212 deletions.
1 change: 0 additions & 1 deletion bindings/pyroot/pythonizations/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ set(cpp_sources
src/GenericPyz.cxx
src/RVecPyz.cxx
src/TClassPyz.cxx
src/TClonesArrayPyz.cxx
src/TMemoryRegulator.cxx
src/TObjectPyz.cxx
src/TTreePyz.cxx
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,18 @@
################################################################################

from . import pythonization
from libROOTPythonizations import AddSetItemTCAPyz


@pythonization('TClonesArray')
def _TClonesArray__setitem__(self, key, val):
raise RuntimeError(
"You can't set items of a TClonesArray! You can however TClonesArray::ConstructedAt() to access an element that is guaranteed to be constructed, and then modify it in-place."
)


@pythonization("TClonesArray")
def pythonize_tclonesarray(klass):
# Parameters:
# klass: class to be pythonized

# Add item setter method
AddSetItemTCAPyz(klass)
klass.__setitem__ = _TClonesArray__setitem__
2 changes: 0 additions & 2 deletions bindings/pyroot/pythonizations/src/PyROOTModule.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ static PyMethodDef gPyROOTMethods[] = {
(char *)"Fully enable the use of TTree::SetBranchAddress from Python"},
{(char *)"BranchPyz", (PyCFunction)PyROOT::BranchPyz, METH_VARARGS,
(char *)"Fully enable the use of TTree::Branch from Python"},
{(char *)"AddSetItemTCAPyz", (PyCFunction)PyROOT::AddSetItemTCAPyz, METH_VARARGS,
(char *)"Customize the setting of an item of a TClonesArray"},
{(char *)"AddPrettyPrintingPyz", (PyCFunction)PyROOT::AddPrettyPrintingPyz, METH_VARARGS,
(char *)"Add pretty printing pythonization"},
{(char *)"GetEndianess", (PyCFunction)PyROOT::GetEndianess, METH_NOARGS, (char *)"Get endianess of the system"},
Expand Down
2 changes: 0 additions & 2 deletions bindings/pyroot/pythonizations/src/PyROOTPythonize.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ PyObject *AddTClassDynamicCastPyz(PyObject *self, PyObject *args);

PyObject *AddTObjectEqNePyz(PyObject *self, PyObject *args);

PyObject *AddSetItemTCAPyz(PyObject *self, PyObject *args);

PyObject *AsRVec(PyObject *self, PyObject *obj);
PyObject *AsRTensor(PyObject *self, PyObject *obj);

Expand Down
172 changes: 0 additions & 172 deletions bindings/pyroot/pythonizations/src/TClonesArrayPyz.cxx

This file was deleted.

3 changes: 0 additions & 3 deletions bindings/pyroot/pythonizations/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ ROOT_ADD_PYUNITTEST(pyroot_pyz_tseqcollection_listmethods tseqcollection_listmet
# TIter pythonisations
ROOT_ADD_PYUNITTEST(pyroot_pyz_titer_iterator titer_iterator.py)

# TClonesArray pythonisations
ROOT_ADD_PYUNITTEST(pyroot_pyz_tclonesarray_itemaccess tclonesarray_itemaccess.py)

# TArray and subclasses pythonizations
ROOT_ADD_PYUNITTEST(pyroot_pyz_tarray_len tarray_len.py)
ROOT_ADD_PYUNITTEST(pyroot_pyz_tarray_getitem tarray_getitem.py)
Expand Down
29 changes: 0 additions & 29 deletions bindings/pyroot/pythonizations/test/tclonesarray_itemaccess.py

This file was deleted.

0 comments on commit 0afffc6

Please sign in to comment.