Skip to content

Allow both 'bool' and 'Bool_t' type conversions to numpy booleans #20192

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 26, 2025
Merged

Allow both 'bool' and 'Bool_t' type conversions to numpy booleans #20192

merged 2 commits into from
Oct 26, 2025
+83 −16

Conversation

@vepadulano
Copy link
Member

Fixes #20081

@vepadulano
[python] Extend dictionary for numpy array interface types
12331f2 
Include all standard C++ fundamental types and the corresponding ROOT C++ types. Connect each of them with the correct Numpy array interface basic type as described at https://numpy.org/doc/stable/reference/arrays.interface.html#object.__array_interface__ .
@vepadulano vepadulano requested a review from guitargeek October 24, 2025 14:09
@vepadulano vepadulano self-assigned this Oct 24, 2025
@vepadulano vepadulano requested a review from dpiparo as a code owner October 24, 2025 14:09
@github-actions
Copy link

github-actions bot commented Oct 24, 2025 •
edited
Loading

Test Results

    22 files      22 suites   3d 16h 53m 31s ⏱️
 3 698 tests  3 697 ✅ 0 💤 1 ❌
79 423 runs  79 422 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit b778016.

♻️ This comment has been updated with latest results.

guitargeek
guitargeek reviewed Oct 25, 2025
View reviewed changes
guitargeek
guitargeek requested changes Oct 25, 2025
View reviewed changes
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.

Thanks a lot for the PR! I would however suggest to keep the number of bytes consistent, if there is no particular reason to not do so.

guitargeek
guitargeek approved these changes Oct 25, 2025
View reviewed changes
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.

Nevermind, my previous change request. I have now remembered that the size is later appended to the dtype string as the result of sizeof.

So this PR looks good to me, except for the failure on Windows, which can be easily avoided by not deleting the file!

dpiparo
dpiparo approved these changes Oct 25, 2025
View reviewed changes
Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

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

lgtm, modulo the small change needed for the win tests

@vepadulano
[python] Extend RDataFrame to Numpy array conversion of boolean types
b778016 
* Use ROOT::RVec as the single data structure to recover all types of values via the Take operation. This helps avoiding issues with vectors of boolean values, whether they are visible as the 'bool' or 'Bool_t' C++ types.
* Add a test that checks both 'bool' and 'Bool_t' types. For the latter, it is required to write a TTree branch.
@vepadulano vepadulano merged commit b3d0539 into root-project:master Oct 26, 2025
24 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@guitargeek guitargeek guitargeek approved these changes

@dpiparo dpiparo dpiparo approved these changes

Assignees

@vepadulano vepadulano

Labels

in:Python Interface in:RDataFrame

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Also consider Bool_t in numpy array conversion

3 participants

@vepadulano @guitargeek @dpiparo