feat: Drop uproot3 for uproot4 for writing ROOT files (#1567)

* Remove all uproot3 dependency and use uproot4 for all reading and writing of ROOT files
* Require uproot>=4.1.1 in 'xmlio' extra
* Require uproot==4.1.1 in lower-bound-requirements.txt
* Remove uproot3 HEAD of dependencies testing workflow
* Add test to ensure pyhf.writexml writes histograms to ROOT files as expected

.github/workflows/dependencies-head.yml

@@ -63,31 +63,6 @@ jobs:
       run: |
         python -m pytest -r sx --ignore tests/benchmarks/ --ignore tests/contrib --ignore tests/test_notebooks.py
 
-  uproot3:
-
-    runs-on: ${{ matrix.os }}
-    strategy:
-      matrix:
-        os: [ubuntu-latest]
-        python-version: [3.8]
-
-    steps:
-    - uses: actions/checkout@v2
-    - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v2
-      with:
-        python-version: ${{ matrix.python-version }}
-    - name: Install dependencies
-      run: |
-        python -m pip install --upgrade pip setuptools wheel
-        python -m pip --no-cache-dir --quiet install --upgrade --editable .[test]
-        python -m pip uninstall --yes uproot3
-        python -m pip install --upgrade git+git://github.com/scikit-hep/uproot3.git
-        python -m pip list
-    - name: Test with pytest
-      run: |
-        python -m pytest -r sx --ignore tests/benchmarks/ --ignore tests/contrib --ignore tests/test_notebooks.py
-
   uproot4:
 
     runs-on: ${{ matrix.os }}

lower-bound-requirements.txt

@@ -6,8 +6,7 @@ jsonschema==3.0.0
 jsonpatch==1.15
 pyyaml==5.1
 # xmlio
-uproot3==3.14.1
-uproot==4.0.0
+uproot==4.1.1
 # minuit
 iminuit==2.4.0
 # tensorflow

setup.py

@@ -8,10 +8,7 @@
     ],
     'torch': ['torch~=1.8'],
     'jax': ['jax~=0.2.8', 'jaxlib~=0.1.58,!=0.1.68'],  # c.f. Issue 1501
-    'xmlio': [
-        'uproot3>=3.14.1',
-        'uproot~=4.0',
-    ],  # uproot3 required until writing to ROOT supported in uproot4
+    'xmlio': ['uproot>=4.1.1'],
     'minuit': ['iminuit>=2.4'],
 }
 extras_require['backends'] = sorted(

src/pyhf/writexml.py

@@ -6,9 +6,7 @@
 import xml.etree.ElementTree as ET
 import numpy as np
 
-# TODO: Move to uproot4 when ROOT file writing is supported
-import uproot3 as uproot
-from uproot3_methods.classes import TH1
+import uproot
 
 from pyhf.mixins import _ChannelSummaryMixin
 
@@ -47,12 +45,12 @@ def _make_hist_name(channel, sample, modifier='', prefix='hist', suffix=''):
     return f"{prefix}{middle}{suffix}"
 
 
-def _export_root_histogram(histname, data):
-    hist = TH1.from_numpy((np.asarray(data), np.arange(len(data) + 1)))
-    hist._fName = histname
-    if histname in _ROOT_DATA_FILE:
-        raise KeyError(f"Duplicate key {histname} being written.")
-    _ROOT_DATA_FILE[histname] = hist
+def _export_root_histogram(hist_name, data):
+    if hist_name in _ROOT_DATA_FILE:
+        raise KeyError(f"Duplicate key {hist_name} being written.")
+    _ROOT_DATA_FILE[hist_name] = uproot.to_writable(
+        (np.asarray(data), np.arange(len(data) + 1))
+    )
 
 
 # https://stackoverflow.com/a/4590052
@@ -236,7 +234,7 @@ def build_sample(spec, samplespec, channelname):
     attrs = {
         'Name': samplespec['name'],
         'HistoName': histname,
-        'InputFile': _ROOT_DATA_FILE._path,
+        'InputFile': _ROOT_DATA_FILE.file_path,
         'NormalizeByTheory': 'False',
     }
     sample = ET.Element('Sample', **attrs)
@@ -255,7 +253,7 @@ def build_sample(spec, samplespec, channelname):
 
 def build_data(obsspec, channelname):
     histname = _make_hist_name(channelname, 'data')
-    data = ET.Element('Data', HistoName=histname, InputFile=_ROOT_DATA_FILE._path)
+    data = ET.Element('Data', HistoName=histname, InputFile=_ROOT_DATA_FILE.file_path)
 
     observation = next((obs for obs in obsspec if obs['name'] == channelname), None)
     _export_root_histogram(histname, observation['data'])
@@ -264,7 +262,7 @@ def build_data(obsspec, channelname):
 
 def build_channel(spec, channelspec, obsspec):
     channel = ET.Element(
-        'Channel', Name=channelspec['name'], InputFile=_ROOT_DATA_FILE._path
+        'Channel', Name=channelspec['name'], InputFile=_ROOT_DATA_FILE.file_path
     )
     if obsspec:
         data = build_data(obsspec, channelspec['name'])

tests/test_export.py

@@ -1,9 +1,12 @@
-import pyhf
-import pyhf.writexml
-import pytest
 import json
-import xml.etree.ElementTree as ET
 import logging
+import xml.etree.ElementTree as ET
+
+import pytest
+import uproot
+
+import pyhf
+import pyhf.writexml
 
 
 def spec_staterror():
@@ -392,9 +395,26 @@ def test_export_data(mocker):
     assert pyhf.writexml._ROOT_DATA_FILE.__setitem__.called
 
 
+def test_export_root_histogram(mocker, tmp_path):
+    """
+    Test that pyhf.writexml._export_root_histogram writes out a histogram
+    in the manner that uproot is expecting and verifies this by reading
+    the serialized file
+    """
+    mocker.patch("pyhf.writexml._ROOT_DATA_FILE", {})
+    pyhf.writexml._export_root_histogram("hist", [0, 1, 2, 3, 4, 5, 6, 7, 8])
+
+    with uproot.recreate(tmp_path.joinpath("test_export_root_histogram.root")) as file:
+        file["hist"] = pyhf.writexml._ROOT_DATA_FILE["hist"]
+
+    with uproot.open(tmp_path.joinpath("test_export_root_histogram.root")) as file:
+        assert file["hist"].values().tolist() == [0, 1, 2, 3, 4, 5, 6, 7, 8]
+        assert file["hist"].axis().edges().tolist() == [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
+        assert file["hist"].name == "hist"
+
+
 def test_export_duplicate_hist_name(mocker):
     mocker.patch('pyhf.writexml._ROOT_DATA_FILE', new={'duplicate_name': True})
-    mocker.patch.object(pyhf.writexml, 'TH1')
 
     with pytest.raises(KeyError):
         pyhf.writexml._export_root_histogram('duplicate_name', [0, 1, 2])