scikit-hep / uproot3

ROOT I/O in pure Python and NumPy.
BSD 3-Clause "New" or "Revised" License
314 stars 67 forks source link

not reading tprofile values #521

Open wiso opened 4 years ago

wiso commented 4 years ago

I am reading a tprofile2d with uproot with the numpy method

The edges are read correctly, but what I got as values seems to be the number of entries for each bin, while for a TProfile I am interested to the average of the quantity I am profiling

jpivarski commented 4 years ago

It's because Uproot 3 makes Python classes that have the same inheritance as the underlying C++ classes, TProfile2D inherits from TH2D, and the values method has not been overridden with profile-specific logic. So the problem is that the method you want hasn't been implemented, but due to historical inheritance issues, it manifests itself as the wrong answer, rather than a "not implemented" error or missing method, as it should.

Let's try to fix this in Uproot 4, because Uproot 3 is quickly approaching deprecation-time. In Uproot 4, the Python classes have a flat hierarchy and so we more fully control which class gets which method. The Uproot 4 TProfile2D has an implementation for edges but not for values:

https://github.com/scikit-hep/uproot4/blob/b3b3e7f784a1363334fcec5f8c03c18ac6a0b643/uproot4/behaviors/TProfile2D.py#L21-L30

But TProfile (1-dimensional) has a carefully worked out implementation for values and errors:

https://github.com/scikit-hep/uproot4/blob/b3b3e7f784a1363334fcec5f8c03c18ac6a0b643/uproot4/behaviors/TProfile.py#L107-L177

with extensive tests that were carefully checked against ROOT's output:

https://github.com/scikit-hep/uproot4/blob/b3b3e7f784a1363334fcec5f8c03c18ac6a0b643/tests/test_0046-histograms-bh-hist.py#L2180-L2823

Probably all that needs to be done is to replace expressions like len(array) with array.shape because handling 2-dimensional profile values/errors should be just like handling 1-dimensional profile values/errors with a 2-dimensional shape. It might be even simpler to turn the 1-dimensional implementation into a helper function, flatten a 2-dimensional array on input and restore its 2-dimensional shape on output, and therefore not even have to touch the inner implementation.

Would you like to try that?

wiso commented 4 years ago

do you mean in uproot4? I can have a look, slowly...

jpivarski commented 4 years ago

I know what you mean. :)

I just gave it a look and refactored the TProfile (1-dimensional) code into helper functions that the TProfile2D uses. I haven't tested the TProfile2D, but if anything, it's probably just a typo or two away from working. If you get a chance, try the master branch with your file.