Skip to content
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

[PyROOT] Do __getattr__ pythonization of TTree in Python #196

Closed
wants to merge 2 commits into from
Closed

[PyROOT] Do __getattr__ pythonization of TTree in Python #196

wants to merge 2 commits into from

Conversation

smuzaffar
Copy link

No description provided.

@guitargeek
[tree] Change return type of TBranch::GetAddress() et al to void*
d91c482 
So far, the `TBranch::GetAddress` method returned a `char *`. This
causes incompatibility with PyROOT, because `char *` is automatically
converted to a Python string, which is the wrong thing to for an
address.

This made the return value unusable in PyROOT. That's a bug that
currently prevents us from reimplementing some pythonizations in native
Python.

Therefore, the return type of `TBranch::GetAddress()` was changed to
`void *` in this release. Even if this change is not backwards
compatible for typing reasons, the returned value is the same.

With this change, the code is also more consistent because
`GetAddress()` and `SetBranchAddress()` use the same type for the
address.
@guitargeek
[PyROOT] Do __getattr__ pythonization of TTree in Python
91c0a57 
There is no need to do this on the C++ size.
@smuzaffar
Copy link
Author

please test for CMSSW_14_0_ROOT6_X

@cmsbuild
Copy link

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for branch cms/master/953770d5da.

@aandvalenzuela, @iarspider, @smuzaffar can you please review it and eventually sign? Thanks.
@rappoccio, @antoniovilela, @sextonkennedy you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link

cmsbuild commented Jan 26, 2024 •
edited
Loading

cms-bot internal usage

@smuzaffar smuzaffar mentioned this pull request Jan 26, 2024
Closed
@cmsbuild
Copy link

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ce122a/37051/summary.html
COMMIT: 91c0a57
CMSSW: CMSSW_14_0_ROOT6_X_2024-01-25-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/root/196/37051/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 1 errors in the following unit tests:

---> test TestDQMServicesDemo had ERRORS

Comparison Summary

Summary:

@iarspider iarspider mentioned this pull request Feb 8, 2024
Merged
@smuzaffar smuzaffar closed this Feb 13, 2024
@guitargeek guitargeek deleted the ttree_getattr branch March 16, 2024 18:29
@guitargeek guitargeek restored the ttree_getattr branch June 7, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
externals-pending orp-pending pending-signatures tests-rejected
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@smuzaffar @cmsbuild @guitargeek