First commit of hist2root.py

[slehti]

Convert 1-dimensional or 2-dimensional hist object into ROOT:TH1D or ROOT.TH2D object, convert counter into a ROOT.TH1D object.

[LovelyBuggies]

@slehti Hi, thanks for this pr. Could you add some python type annotations to pass GitHub Ci? And it would be good if you can give tests or a notebook for this feature.

[henryiii]

I can help with type annotations and setting up ROOT in the tests. I'll review soon.

[henryiii]

I would hope hist would be a dependency of coffea, not depending on coffea. This should be done a different way - either optionally importing it, or better yet, if it could be done via a Protocol (check for methods and call if they exist, basically). I'll go through and do a bit of CI / type annotations, then we can revisit this part.

[henryiii]

Did you test this implementation by hand? Static typing revealed h.axes()[...], which is invalid - axes is a tuple, which can't be called. That's an easy fix, but then I ran across h.keys(), which does not exist for a histogram, histogram's don't have keys (what would that even be? The axes names, perhaps? Numbers?). Histograms also don't have labels by default (only axes do), but that probably was supposed to be "name"? Also .values doesn't have a overflow or sumw2 argument. Maybe I'm misunderstanding what this does?

src/hist/hist2root.py:55: error: "BaseHist" has no attribute "keys"
src/hist/hist2root.py:57: error: "BaseHist" has no attribute "keys"
src/hist/hist2root.py:64: error: "BaseHist" has no attribute "label"
src/hist/hist2root.py:67: error: "BaseHist" has no attribute "label"
src/hist/hist2root.py:76: error: Unexpected keyword argument "sumw2" for "values" of "Histogram"
src/hist/hist2root.py:76: error: Unexpected keyword argument "overflow" for "values" of "Histogram"
src/hist/hist2root.py:83: error: "BaseHist" has no attribute "label"
src/hist/hist2root.py:86: error: "BaseHist" has no attribute "label"
src/hist/hist2root.py:102: error: Unexpected keyword argument "sumw2" for "values" of "Histogram"
src/hist/hist2root.py:102: error: Unexpected keyword argument "overflow" for "values" of "Histogram"

[henryiii]

Also, please always make a new branch for making a PR. It's a bit more irritating to have to write git push [email protected]:slehti/hist.git HEAD:main than it is to just to write git push, which could be done only if the branch name is unique. Okay by me, but does make it easier to help, so just a future recommendation, especially if you contribute to other repos. :)

[henryiii]

This is confusing. If this has an empty hist.label, then it uses that. If it's not empty, then it uses the passed name instead, even if that is empty. I really don't think that's what you intended. Instead of this structure, which is easy to get wrong, try this:

Suggested change

	name = hist.label
	if len(name) > 0:
	name = hname
	name = hname or hist.label

That will set hname if not empty, otherwise it will try to use hist.label. Or maybe you meant the reverse, and hname was a fallback - in that case, name = hist.label or hname.

[henryiii]

A general point: we should split up PyROOT and coffea conversion. Most users will be converting PyROOT and won't care about coffea. I'll also have to look at coffea a bit more to see how that's set up. So I'd focus on PyROOT for now. I'd then call the file pyroot.py, so hist.pyroot.convert(h) would be the function, which I think reads well. I should have mentioned this in #262.

We should soon be able to save boost-histogram and Hist to files without using PyROOT at all, but using uproot4 instead. That's part of the reason for adding "py" to the name here.

Thanks for the PR! Please address what you can (especially things where I'm not sure the original intent), and ideally try to make sure it runs locally on something (pick some random use case). Then I'll help with whatever you can't do (just will be a little slower getting done, most likely).