Skip to content

[CPyCppyy] Drop __array__ from std::vector pythonizations #18208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2025

Conversation

aaronj0
Copy link
Contributor

@aaronj0 aaronj0 commented Mar 31, 2025

The addition of the __array__ utility to std::vector Python proxies causes a bug where the resulting array is a single dimension, causing loss of data when converting to numpy arrays, for >1dim vectors. The recursive nature of this
function VectorArray, passes each subarray (pydata) to the next call and only the final buffer is cast to a lowlevel view and resized (in VectorData), resulting in only the first 1D array to be returned.

Since this C++ pythonization was added with the upgrade in 6.32, and is only defined and used recursively, the safe option is to disable this function and no longer add it. It is temporarily removed to prevent errors due to -Wunused-function

Fixes: #17729

Copy link

github-actions bot commented Apr 1, 2025

Test Results

    17 files      17 suites   4d 3h 24m 56s ⏱️
 2 737 tests  2 724 ✅ 0 💤 13 ❌
44 947 runs  44 931 ✅ 0 💤 16 ❌

For more details on these failures, see this check.

Results for commit 55e8ca6.

♻️ This comment has been updated with latest results.

The addition of the __array__ utility to std::vector Python proxies causes a
bug where the resulting array is a single dimension, causing loss of data when
converting to numpy arrays, for >1dim vectors. The recursive nature of this
function, passes each subarray (pydata) to the next call and only the final
buffer is cast to a lowlevel view and resized (in VectorData), resulting in
only the first 1D array to be returned. See root-project#17729

Since this C++ pythonization was added with the upgrade in 6.32, and is only
defined and used recursively, the safe option is to disable this function and
no longer add it. It is temporarily removed to prevent errors due to -Wunused-function
@aaronj0 aaronj0 changed the title [TEST PR] Drop __array__ from std::vector pythonizations in CPyCppyy [CPyCppyy] Drop __array__ from std::vector pythonizations Apr 2, 2025
Copy link
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you very much for the investigation and finding just the right fix!

I'll also backport it to the 6.34 branch, so if there is a patch release of 6.34 before the upcoming 6.36, our users will also benefit from this fix.

@guitargeek guitargeek merged commit fcc727f into root-project:master Apr 2, 2025
13 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Python Interface] Regression: can't properly convert a vector to 2D/3D numpy array
2 participants