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.

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

Error in reading ROOT files containing TMatrix and TVector #383

Closed
nfoppiani opened this issue Jun 29, 2021 · 7 comments · Fixed by #384
Closed

Error in reading ROOT files containing TMatrix and TVector #383

nfoppiani opened this issue Jun 29, 2021 · 7 comments · Fixed by #384
Labels
bug (unverified) The problem described would be a bug, but needs to be triaged

Comments

@nfoppiani
Copy link

I have been trying to read a root file containing TVector and TMatrix, the idea being following https://stackoverflow.com/questions/65833805/how-do-i-read-a-tmatrixt-with-uproot-in-python
I tried with the following code:

import uproot
file_u = uproot.open('efficiency.root')
file_u[file_u.keys()[0]]
The last line to get the first object in the file, but I get the following error:


TypeError Traceback (most recent call last)
in
----> 1 file_u[file_u.keys()[0]]

~/.local/lib/python3.9/site-packages/uproot/reading.py in getitem(self, where)
2043
2044 else:
-> 2045 return self.key(where).get()
2046
2047 @Property

~/.local/lib/python3.9/site-packages/uproot/reading.py in get(self)
2434
2435 try:
-> 2436 out = cls.read(chunk, cursor, context, self._file, selffile, parent)
2437
2438 except uproot.deserialization.DeserializationError:

~/.local/lib/python3.9/site-packages/uproot/model.py in read(cls, chunk, cursor, context, file, selffile, parent, concrete)
1194
1195 elif version is not None:
-> 1196 versioned_cls = cls.new_class(file, version)
1197
1198 elif context.get("in_TBranch", False):

~/.local/lib/python3.9/site-packages/uproot/model.py in new_class(cls, file, version)
1130
1131 if streamer is not None:
-> 1132 versioned_cls = streamer.new_class(file)
1133 versioned_cls.class_streamer = streamer
1134 cls.known_versions[streamer.class_version] = versioned_cls

~/.local/lib/python3.9/site-packages/uproot/streamers.py in new_class(self, file)
370 class_name = uproot.model.classname_encode(self.name, self.class_version)
371 classes = uproot.model.maybe_custom_classes(file.custom_classes)
--> 372 return uproot.deserialization.compile_class(
373 file, classes, class_code, class_name
374 )

~/.local/lib/python3.9/site-packages/uproot/deserialization.py in compile_class(file, classes, class_code, class_name)
78 setattr(uproot.dynamic, out.name, out)
79
---> 80 behaviors = tuple(_yield_all_behaviors(out, c))
81 exclude = tuple(
82 bad for cls in behaviors if hasattr(cls, "no_inherit") for bad in cls.no_inherit

~/.local/lib/python3.9/site-packages/uproot/deserialization.py in _yield_all_behaviors(cls, c)
32
33 def _yield_all_behaviors(cls, c):
---> 34 behavior_cls = uproot.behavior_of(uproot.model.classname_decode(cls.name)[0])
35 if behavior_cls is not None:
36 yield behavior_cls

~/.local/lib/python3.9/site-packages/uproot/init.py in behavior_of(classname)
227 return globals().get(name)
228 else:
--> 229 return globals().get(name)(specialization)
230
231

TypeError: 'NoneType' object is not callable
I run uproot version '4.0.10' and Python 3.9.5
If I open the same file with pyROOT I can read the TMatrix content, and it is shown as <cppyy.gbl.TMatrixT object at 0x557c36963df0>

@nfoppiani nfoppiani added the bug (unverified) The problem described would be a bug, but needs to be triaged label Jun 29, 2021
@jpivarski jpivarski linked a pull request Jun 29, 2021 that will close this issue
@jpivarski
Copy link
Member

I know what happened: the uproot.behavior_of function tries to pass template specializations to behavior classes, and TMatrix/TVector are templated. However, no specializations for these classes have been written in Python, so it was trying to pass that specialization to None.

I've moved the function (as I've been wanting to do for some time) and fixed the bug, probably, but I don't have any examples of files with a TMatrix or TVector in it. I could add such a test to the PR if you give me a few lines of PyROOT that produces one (I'm guessing that this is simpler than adding a real-data file to scikit-hep-testdata). Otherwise, could you test PR #384 with your file before I merge it and make a new release for it? Thanks!

(My preference is to add a test based on a small file made dynamically by PyROOT, if you have code for that offhand. Then the test will stay in the codebase but I won't have to mess with scikit-hep-testdata. ROOT is already a dependency for testing.)

@agoose77
Copy link
Collaborator

agoose77 commented Jun 30, 2021

@nfoppiani does this reproduce the bug for you?

import ROOT
import uproot

path = "/tmp/test-file.root"
key = "mat"

# Write test file
f = ROOT.TFile(path, "RECREATE")
mat = ROOT.TMatrixD(3, 3)
mat[0, 1] = 4
mat[1, 0] = 8
mat[2, 2] = 3
mat.Write(key")
f.Close()

    
# Read test file
with uproot.open(path) as f:
    mat = f[key]

@jpivarski
Copy link
Member

If the name of this class is "TMatrixD", then the template specialization that's breaking must be in a superclass. I'll try using this as a test, thanks! (If it has cls is None but specialization is not None, than it's a reproducer.)

@agoose77
Copy link
Collaborator

(If it has cls is None but specialization is not None, than it's a reproducer.)

This should be the case - we don't define a behaviour for TMatrix AFAICT, so the cls should be None.

@nfoppiani
Copy link
Author

Thanks, @agoose77. Indeed I obtain the same error.
The pyROOT object mat in your code is of type <cppyy.gbl.TMatrixT<double> object at 0x55e22a3b73e0>, which is the same as the one I was encountering before, aside from the "specialization" to double.

@nfoppiani
Copy link
Author

Otherwise, could you test PR #384 with your file before I merge it and make a new release for it? Thanks!

@jpivarski I tested PR #384 with my file and the example provided by @agoose77: it worked in both cases, and I could access the data by calling .member("fElements").

I think the dynamically generated example is good enough; otherwise, I can give you the file I was using, but it adds more complication than there needs to be, in my opinion.

jpivarski added a commit that referenced this issue Jun 30, 2021
* Fixed #383 and moved behavior_of to its own module.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* And documentation for the new module.

* Fix precommits.

* Added a test. (It would trigger the bug.)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@jpivarski
Copy link
Member

The fix is in Uproot 4.0.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug (unverified) The problem described would be a bug, but needs to be triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants