root icon indicating copy to clipboard operation
root copied to clipboard

[cppyy] fix iadd pythonization for np/array buffers

Open aaronj0 opened this issue 6 months ago • 2 comments

Backport of https://github.com/wlav/CPyCppyy/pull/51 Fixes https://github.com/root-project/root/issues/18768

Currently, the iadd implementation for std::vector in Pythonize.cxx works perfectly with Python lists and iterables for which we have an ItemGetter. But the code path for numpy/array buffers do not. The difference between x+=y and x.__iadd__(y) leads to unintuitive behaviour: if the intent is to obtain the resulting iterator when calling insert, the user can do d.insert(d.end(), a) which makes more sense than expecting it from d+=a. When the user does call d+=a, since __iadd__ is overriden, operation does not end with the call to VectorIAdd which should just return the iterator, and reflect the change by insertion on d. Instead, it ends with d = d.__iadd__(a) which reassigns d to the returned iterator, losing the vector in the process. This PR fixes this by returning self in our pythonization.

aaronj0 avatar Jun 09 '25 17:06 aaronj0

Test Results

    20 files      20 suites   3d 16h 35m 20s ⏱️  3 013 tests  3 011 ✅ 0 💤 2 ❌ 58 672 runs  58 670 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit d81e2c31.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Jun 09 '25 18:06 github-actions[bot]

Thanks for these changes. Can we add a (some) test(s)?

dpiparo avatar Jun 10 '25 05:06 dpiparo