[Python] Move the TTree branch pythonization to Python - #22512
Draft
guitargeek wants to merge 4 commits into
Draft
[Python] Move the TTree branch pythonization to Python#22512guitargeek wants to merge 4 commits into
guitargeek wants to merge 4 commits into
Conversation
Test Results 23 files 23 suites 3d 11h 26m 17s ⏱️ For more details on these failures, see this check. Results for commit a867924. ♻️ This comment has been updated with latest results. |
vepadulano
reviewed
Jun 8, 2026
guitargeek
force-pushed
the
ttree_pyz
branch
2 times, most recently
from
August 8, 2026 21:44
9024654 to
858b2fe
Compare
guitargeek
changed the title
guitargeek
force-pushed
the
ttree_pyz
branch
2 times, most recently
from
August 8, 2026 22:26
cc12b77 to
a990e87
Compare
guitargeek
marked this pull request as draft
August 8, 2026 22:26
LowLevelView.reshape() compared the sum of the dimensions of the old and the new shape, where it should compare the number of elements, which is their product. Reshaping a six element array to (2, 3) was rejected on the grounds that 6 != 2 + 3, while reshaping it to (5, 1) was accepted. The check is skipped for arrays of unknown size, which is why this went unnoticed: those are the ones that typically get reshaped. Also hoist the computation of the new size out of the comparison, so that a non-integer entry in the shape is reported as such even when the old size is unknown. 🤖 Done with the help of AI
A multi-dimensional LowLevelView is not just a Py_buffer with more
entries in its shape. Indexing it projects sub-views through a converter
that is built for the element type with one dimension peeled off, and it
is that converter, not the shape, that decides what v[i] returns.
reshape() only rewrote shape and strides, leaving the converter of the
one-dimensional view it started from in place. The result claimed the
requested shape but did not behave like it:
v = ll.cast['float*'](addr)
v.reshape((2, 3))
v.shape # (2, 3)
v[0] # 20.0, an element, where a sub-view is expected
np.asarray(v) # [[20, 21, 22], [21, 22, 23]], from bad strides
v[1,2] # segfault, reading data as a pointer
Re-create the view through the creator that made it, which is the same
mechanism slicing already uses, and take over the result. That gets the
converter, the item size, the strides and the fixed-size flag right by
construction, rather than by duplicating that logic here.
Only rank-1 views can be grown this way. Their data is by construction a
flat block, whereas a view that is already multi-dimensional carries a
layout that its shape does not describe: T** data is an array of row
pointers, not a contiguous block, and re-creating those as flat arrays
would silently read the pointers as values. For those, reshape() keeps
filling in the dimensions in place, as before.
Reshaping back down to one dimension has to restore the element
converter, or the view keeps handing out sub-views it no longer has the
rank for.
🤖 Done with the help of AI
Reading a value of a known type from a known address is something cppyy
does constantly, but it was not reachable from Python. The pieces that
come closest each fall short:
* ll.cast['T*'](addr) goes through a pointer converter, so it cannot
express the shape of an array, and for char* it produces a Python
str, throwing the buffer away.
* bind_object only handles class types.
* as_memoryview and the CreateLowLevelView overloads need the element
type resolved by the caller, which is exactly the work the converter
machinery already does from a type name.
Expose that machinery directly: value_from_memory(type_name, address,
dims=None) creates the converter for the type name and reads through it,
optionally with the shape of an array, and returns whatever cppyy would
have returned had it read the same memory itself: a proxy for a class
type, a Python value for a builtin, a LowLevelView for an array.
This is for code that holds a type name and an address but no C++ entity
to read them from, such as a framework describing its own data layout.
Reading a TTree leaf is the case this was written for.
🤖 Done with the help of AI
TTreePyz.cxx implemented the tree.branch attribute syntax and the TTree::Branch overload dispatch in C++, next to a Python file holding the rest of the TTree pythonization. Both are pythonizations: they belong with the others, where they can be read and changed without a rebuild. The parts that kept them in C++ are gone now. Wrapping a leaf needs a converter for a type name and, for arrays, a shape; that is what ll.value_from_memory does. Matching the Branch overloads needs argument inspection, which Python does natively and more legibly than PyArg_ParseTuple. What is left is TBranch::GetAddress() and TBranchElement::GetObject(), which return char* and so reach Python as a str with the pointer value gone. Neither class offers a void* accessor and fAddress is protected, so those two are declared as intptr_t wrappers through the interpreter, on first branch access rather than at import, so that sessions that never touch a branch do not pay for them. The two-tuple protocol between GetBranchAttr() and __getattr__, where C++ returned an address and a type name for Python to cast, is gone with it: __getattr__ now returns the value. Which address the T** overloads of Branch want is not something the Python side can work out on its own: a proxy for an object holds the object pointer, while a proxy for a reference to a pointer holds the address of the caller's pointer, and only CPyCppyy knows which is which. Restating that rule here would silently bind the branch to the proxy's own memory in the second case, so a third helper taking a T** is declared instead, and cppyy fills it in the way it does for any other C++ function taking one. ttree_branch.py covers it, asserting on the branch address before filling, since filling through a branch bound to a temporary proxy is undefined. Its new types are declared separately from MyStruct, which TreeHelper.h declares too, so that the redefinition does not reject them along with it. 🤖 Done with the help of AI
guitargeek
added a commit
to guitargeek/cppjit
that referenced
this pull request
Sep 3, 2026
LowLevelView.reshape() verified sizes by comparing the sum of the dimensions instead of the number of elements, their product: reshaping a five element array to (2, 3) was accepted because 5 == 2 + 3, while (1, 5) was rejected. The check is skipped for arrays of unknown size, the ones that typically get reshaped, which is why this went unnoticed. A correct size check is not enough, though: the strides and the converter projecting sub-views are chosen for the rank and layout of the view's C++ type and are not re-derived when reshaping. Any rank change produced a view reading garbage, and even the identity reshape of a fixed int[3][5] corrupted its strides. Reshape therefore now does what it is actually used for: providing the extent of dimensions the type leaves open, such as the size of an array behind a pointer. The rank must match, and only an unknown or empty dimension may be set, with -1 still standing for "unknown"; anything else raises ValueError, including dimensions whose byte size would overflow. The byte length is counted in strides of the outermost dimension as the creators count it, which for views with an itemsize override (const char*[], notably) differs from the itemsize, and the strides themselves are left as the creator laid them down. Also share the "fake max" marking an unknown outermost dimension between the creators and reshape (it was rederived from the itemsize, mistaking the unknown size of row-pointer and itemsize-overridden views for a known one), give the shape property a proper setter (reshape was installed directly despite its mismatching signature, so assignment misreported its result and deletion crashed), refuse to reshape a view without dimensions instead of reading through its null strides, and check allocations in the shape getter. Same issue as root-project/root#22512 (159ee7a1); its rework of rank-changing reshapes is left for a follow-up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement the TTree pythonizations completely in Python.