scikit-hep / uproot3-methods

Pythonic behaviors for non-I/O related ROOT classes.
BSD 3-Clause "New" or "Revised" License
21 stars 28 forks source link

Add from_numpy method to TH2 #55

Closed rob-tay closed 5 years ago

rob-tay commented 5 years ago

Adds ability to write 2D histograms created using np.histogram2d.

Changes:

  1. from_numpy() method added to classes.TH2
  2. Necessary additions to convert.towriteable() for directly writing to file
f = uproot.create("file.root")
data = np.random.normal(0, 1, (10000, 2))
f['hist']= np.histogram2d(data[:,0], data[:,1], bins=(50, 50), range=((-5, 5), (-5, 5)))
f.close()
jpivarski commented 5 years ago

Thanks, @rob-tay!

@reikdas, does this work with your developments in file-writing? (You added a superclass TH for multi-dimensional histograms—is anything in this PR redundant with that or needs to be changed to work with that? Or is it consistent and just adds to what you've done?)

reikdas commented 5 years ago

I added a comment about a possible redundant check. Apart from that looks great! :)

jpivarski commented 5 years ago

Maybe the explicit TH2 check is redundant after the TH check, and if so, it should probably be removed.

Does this require a particular combination of uproot and uproot-methods versions to function properly? That is, will any combinations that are allowed by the version constraints in setup.py cause mysterious exceptions, rather than just NotImplementedError if you're using a version that doesn't have the TH2 handling yet?

Do we need to synchronize new version constraints between uproot and uproot-methods to get this feature? (Hopefully not!)

rob-tay commented 5 years ago

Does this require a particular combination of uproot and uproot-methods versions to function properly? That is, will any combinations that are allowed by the version constraints in setup.py cause mysterious exceptions, rather than just NotImplementedError if you're using a version that doesn't have the TH2 handling yet?

Hmm, yeah this will require uproot 3.5.1 which adds the uproot.write.objects.TH.TH class. If using a previous version then I think this will result in a ModuleNotFoundError when importlib is used to import the TH class

jpivarski commented 5 years ago

Okay, then for safety, I'll coordinate the inclusion of this PR with the next semi-major release (uproot 3.6).

I'm also revamping the lazy-array implementation—replacing uproot's custom LazyArray class with one constructed from awkward's ChunkedArray of VirtualArrays. The lazy-array making methods will have slightly altered parameter lists, which suggests a new semi-major release. All of this will have to go in before an uproot HATS next week.