Skip to content

Conversation

@aaronj0
Copy link
Contributor

@aaronj0 aaronj0 commented Jun 9, 2025

Backport of wlav/CPyCppyy#51
Fixes #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 aaronj0 self-assigned this Jun 9, 2025
@github-actions
Copy link

github-actions bot commented Jun 9, 2025

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 d81e2c3.

♻️ This comment has been updated with latest results.

@aaronj0 aaronj0 requested a review from guitargeek June 9, 2025 20:40
@dpiparo
Copy link
Member

dpiparo commented Jun 10, 2025

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

@aaronj0 aaronj0 force-pushed the fix-vec-iadd branch 3 times, most recently from fe6a7aa to 2a01ccc Compare June 20, 2025 12:58
@aaronj0 aaronj0 closed this Jun 21, 2025
@aaronj0 aaronj0 added the clean build Ask CI to do non-incremental build on PR label Jun 21, 2025
@aaronj0 aaronj0 reopened this Jun 21, 2025
@aaronj0 aaronj0 force-pushed the fix-vec-iadd branch 2 times, most recently from 510148c to 725b8cb Compare June 21, 2025 09:05
@dpiparo dpiparo merged commit 99975b2 into root-project:master Jun 21, 2025
22 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean build Ask CI to do non-incremental build on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression in i-adding array.array to std::vector in PyROOT

2 participants