From 4a3c98516a49abc98aa00a7088ad0da87934b72f Mon Sep 17 00:00:00 2001
From: Alexander Held <alexander.held@cern.ch>
Date: Thu, 9 Feb 2023 17:20:33 +0100
Subject: [PATCH 1/7] fix: raise error when converting xml workspaces without
 data

---
 src/pyhf/readxml.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/pyhf/readxml.py b/src/pyhf/readxml.py
index 165d51685c..8a1f9e1573 100644
--- a/src/pyhf/readxml.py
+++ b/src/pyhf/readxml.py
@@ -279,6 +279,11 @@ def process_data(
     histopath = sample.attrib.get('HistoPath', histopath)
     histoname = sample.attrib['HistoName']
 
+    if inputfile == "" or histopath == "" or histoname == "":
+        raise NotImplementedError(
+            "conversion of workspaces without data is not currently supported, see issue #566"
+        )
+
     data, _ = import_root_histogram(resolver, inputfile, histopath, histoname)
     return data
 

From b7cc175e0480481fafe797973aedacc5415150f3 Mon Sep 17 00:00:00 2001
From: Alexander Held <alexander.held@cern.ch>
Date: Thu, 9 Feb 2023 17:25:32 +0100
Subject: [PATCH 2/7] flip words for better phrasing

---
 src/pyhf/readxml.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pyhf/readxml.py b/src/pyhf/readxml.py
index 8a1f9e1573..918297d1c9 100644
--- a/src/pyhf/readxml.py
+++ b/src/pyhf/readxml.py
@@ -281,7 +281,7 @@ def process_data(
 
     if inputfile == "" or histopath == "" or histoname == "":
         raise NotImplementedError(
-            "conversion of workspaces without data is not currently supported, see issue #566"
+            "conversion of workspaces without data is currently not supported, see issue #566"
         )
 
     data, _ = import_root_histogram(resolver, inputfile, histopath, histoname)

From 5d259b1ae3a153e5ed318427e6b07b05b89d5dc1 Mon Sep 17 00:00:00 2001
From: Alexander Held <alexander.held@cern.ch>
Date: Thu, 9 Feb 2023 17:45:53 +0100
Subject: [PATCH 3/7] histopath can be empty string

---
 src/pyhf/readxml.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pyhf/readxml.py b/src/pyhf/readxml.py
index 918297d1c9..55c592bb43 100644
--- a/src/pyhf/readxml.py
+++ b/src/pyhf/readxml.py
@@ -279,7 +279,7 @@ def process_data(
     histopath = sample.attrib.get('HistoPath', histopath)
     histoname = sample.attrib['HistoName']
 
-    if inputfile == "" or histopath == "" or histoname == "":
+    if inputfile == "" or histoname == "":
         raise NotImplementedError(
             "conversion of workspaces without data is currently not supported, see issue #566"
         )

From ebd7f608c52371f93c547158711b4ed26367544b Mon Sep 17 00:00:00 2001
From: Alexander Held <alexander.held@cern.ch>
Date: Wed, 15 Feb 2023 18:03:40 +0100
Subject: [PATCH 4/7] add test

---
 tests/test_import.py                          |  14 ++
 .../config/HistFactorySchema.dtd              | 160 ++++++++++++++++++
 .../config/example.xml                        |   8 +
 .../config/example_channel.xml                |  16 ++
 4 files changed, 198 insertions(+)
 create mode 100644 tests/test_import/xmlimport_noChannelDataPaths/config/HistFactorySchema.dtd
 create mode 100644 tests/test_import/xmlimport_noChannelDataPaths/config/example.xml
 create mode 100644 tests/test_import/xmlimport_noChannelDataPaths/config/example_channel.xml

diff --git a/tests/test_import.py b/tests/test_import.py
index 4e52232570..2cff4fa1c7 100644
--- a/tests/test_import.py
+++ b/tests/test_import.py
@@ -491,6 +491,20 @@ def test_import_noChannelData(mocker, datadir):
         pyhf.readxml.parse(basedir.joinpath("config/example.xml"), basedir)
 
 
+def test_import_noChannelDataPaths(mocker, datadir):
+    _data = [0.0]
+    _err = [1.0]
+    mocker.patch('pyhf.readxml.import_root_histogram', return_value=(_data, _err))
+
+    basedir = datadir.joinpath("xmlimport_noChannelDataPaths")
+    with pytest.raises(NotImplementedError) as excinfo:
+        pyhf.readxml.parse(basedir.joinpath("config/example.xml"), basedir)
+    assert (
+        'conversion of workspaces without data is currently not supported, see issue #566'
+        in str(excinfo.value)
+    )
+
+
 def test_import_missingPOI(mocker, datadir):
     _data = [0.0]
     _err = [1.0]
diff --git a/tests/test_import/xmlimport_noChannelDataPaths/config/HistFactorySchema.dtd b/tests/test_import/xmlimport_noChannelDataPaths/config/HistFactorySchema.dtd
new file mode 100644
index 0000000000..a1dbc10333
--- /dev/null
+++ b/tests/test_import/xmlimport_noChannelDataPaths/config/HistFactorySchema.dtd
@@ -0,0 +1,160 @@
+
+<!-- The top level combination spec -->
+<!-- OutputFilePrefix: Prefix to the output root file to be created (inspection histograms) -->
+<!-- Mode: Type of the analysis -->
+<!ELEMENT Combination (Function*,Input+,Measurement*)>
+<!ATTLIST Combination
+        OutputFilePrefix         CDATA            #REQUIRED
+        Mode                     CDATA            #IMPLIED
+>
+
+<!-- Input files detailing the channels. One channel per file -->
+<!ELEMENT Function  EMPTY>
+<!ATTLIST Function
+     Name                 CDATA         #REQUIRED
+     Expression           CDATA         #REQUIRED
+     Dependents           CDATA         #REQUIRED
+>
+
+<!-- Input files detailing the channels. One channel per file -->
+<!ELEMENT Input (#PCDATA) >
+
+<!-- Configuration for each measurement -->
+<!-- Name: to be used as the heading in the table -->
+<!-- Lumi: the luminosity of the measurement -->
+<!-- LumiRelErr: the relative error known for the lumi -->
+<!-- BinLow: the lowest bin number used for the measurement (inclusive) -->
+<!-- BinHigh: the highest bin number used for the measurement (exclusive) -->
+<!-- Mode: type of the measurement (a closed list of ...) -->
+<!-- ExportOnly: if "True" skip fit, only export model -->
+<!ELEMENT Measurement (POI,ParamSetting*,ConstraintTerm*) >
+<!ATTLIST Measurement
+        Name              CDATA            #REQUIRED
+        Lumi              CDATA            #REQUIRED
+        LumiRelErr        CDATA            #REQUIRED
+        BinLow            CDATA            #IMPLIED
+        BinHigh           CDATA            #IMPLIED
+        Mode              CDATA            #IMPLIED
+        ExportOnly        CDATA            #IMPLIED
+>
+
+<!-- Specify what you are measuring. Corresponds to the name specified in the construction
+of the model in the channel setup. Typically the NormFactor for xsec measurements -->
+<!ELEMENT POI (#PCDATA) >
+
+<!-- Specify what parameters are fixed, or have particular value -->
+<!-- Val: set the value of the parameter -->
+<!-- Const: set this parameter constant -->
+<!ELEMENT ParamSetting (#PCDATA)>
+<!ATTLIST ParamSetting
+        Val               CDATA           #IMPLIED
+        Const             CDATA           #IMPLIED
+>
+
+<!-- Specify an alternative shape to use for given constraint terms (Gaussian is used if this is not specified) -->
+<!-- Type: can be Gamma or Uniform -->
+<!-- RelativeUncertainty: relative uncertainty on the shape -->
+<!ELEMENT ConstraintTerm (#PCDATA)>
+<!ATTLIST ConstraintTerm
+        Type                  CDATA       #REQUIRED
+        RelativeUncertainty   CDATA       #IMPLIED
+>
+
+<!-- Top element for channels. InputFile, HistoName and HistoPath
+can be set at this level in which case they will become defaul to
+all subsequent elements. Otherwise they can be set in individual
+subelements -->
+<!ELEMENT Channel (Data*,StatErrorConfig*,Sample+)>
+<!-- InputFile: input file where the input histogram can be found (use abs path) -->
+<!-- HistoPath: the path (within the root file) where the histogram can be found -->
+<!-- HistoName: the name of the histogram to be used for this (and following in not overridden) item -->
+<!ATTLIST Channel
+        Name              CDATA            #REQUIRED
+        InputFile         CDATA            #IMPLIED
+        HistoPath         CDATA            #IMPLIED
+        HistoName         CDATA            #IMPLIED
+>
+
+<!-- Data to be fit. If you don't provide it, Asimov data will be created -->
+<!-- InputFile: any item set here will override the configuration for the subelements.
+For this element there is no sublemenents so the setting will only have local effects -->
+<!ELEMENT Data EMPTY>
+<!ATTLIST Data
+        InputFile         CDATA            #IMPLIED
+        HistoPath         CDATA            #IMPLIED
+        HistoName         CDATA            #IMPLIED
+>
+
+<!ELEMENT StatErrorConfig EMPTY>
+<!ATTLIST StatErrorConfig
+        RelErrorThreshold      CDATA            #IMPLIED
+        ConstraintType         CDATA            #IMPLIED
+>
+
+
+<!-- Sample elements are made up of systematic variations -->
+<!ELEMENT Sample (StatError | HistoSys | OverallSys | ShapeSys | NormFactor | ShapeFactor)*>
+<!ATTLIST Sample
+        Name              CDATA            #REQUIRED
+        InputFile         CDATA            #IMPLIED
+        HistoName         CDATA            #IMPLIED
+        HistoPath         CDATA            #IMPLIED
+        NormalizeByTheory      CDATA       #IMPLIED
+>
+
+<!-- Systematics for which the variation is provided by histograms -->
+<!ELEMENT StatError EMPTY>
+<!ATTLIST StatError
+     Activate          CDATA            #REQUIRED
+          HistoName         CDATA            #IMPLIED
+          InputFile         CDATA            #IMPLIED
+          HistoPath         CDATA            #IMPLIED
+>
+
+<!ELEMENT HistoSys EMPTY>
+<!ATTLIST HistoSys
+        Name              CDATA            #REQUIRED
+        InputFile         CDATA            #IMPLIED
+        HistoFileHigh     CDATA            #IMPLIED
+        HistoPathHigh     CDATA            #IMPLIED
+        HistoNameHigh     CDATA            #IMPLIED
+        HistoFileLow      CDATA            #IMPLIED
+        HistoPathLow      CDATA            #IMPLIED
+        HistoNameLow      CDATA            #IMPLIED
+>
+
+<!-- Systematics for which the variation is provided by simple overall scaling -->
+<!ELEMENT OverallSys EMPTY>
+<!ATTLIST OverallSys
+        Name              CDATA            #REQUIRED
+        High              CDATA            #REQUIRED
+        Low               CDATA            #REQUIRED
+>
+
+<!-- Systematics for which the variation is provided by simple overall scaling -->
+<!ELEMENT ShapeSys EMPTY>
+<!ATTLIST ShapeSys
+          Name              CDATA            #REQUIRED
+          HistoName         CDATA            #REQUIRED
+          HistoPath         CDATA            #IMPLIED
+          InputFile         CDATA            #IMPLIED
+          ConstraintType    CDATA            #IMPLIED
+>
+
+<!-- Scaling factor, which may be the parameter of interest for cross section measurements-->
+<!ELEMENT NormFactor EMPTY>
+<!ATTLIST NormFactor
+        Name              CDATA            #REQUIRED
+        Val               CDATA            #REQUIRED
+        High              CDATA            #REQUIRED
+        Low               CDATA            #REQUIRED
+        Const             CDATA            #IMPLIED
+>
+
+
+<!-- Systematics for which the variation is provided by simple overall scaling -->
+<!ELEMENT ShapeFactor EMPTY>
+<!ATTLIST ShapeFactor
+          Name              CDATA            #REQUIRED
+>
+
diff --git a/tests/test_import/xmlimport_noChannelDataPaths/config/example.xml b/tests/test_import/xmlimport_noChannelDataPaths/config/example.xml
new file mode 100644
index 0000000000..9e87233426
--- /dev/null
+++ b/tests/test_import/xmlimport_noChannelDataPaths/config/example.xml
@@ -0,0 +1,8 @@
+<!DOCTYPE Combination  SYSTEM 'HistFactorySchema.dtd'>
+<Combination OutputFilePrefix="./results/example" >
+  <Input>./config/example_channel.xml</Input>
+  <Measurement Name="GaussExample" Lumi="1." LumiRelErr="0.1" >
+    <POI>SigXsecOverSM</POI>
+    <ParamSetting Const="True">Lumi alpha_syst1</ParamSetting>
+  </Measurement>
+</Combination>
diff --git a/tests/test_import/xmlimport_noChannelDataPaths/config/example_channel.xml b/tests/test_import/xmlimport_noChannelDataPaths/config/example_channel.xml
new file mode 100644
index 0000000000..ca48ac5c28
--- /dev/null
+++ b/tests/test_import/xmlimport_noChannelDataPaths/config/example_channel.xml
@@ -0,0 +1,16 @@
+<!DOCTYPE Channel  SYSTEM 'HistFactorySchema.dtd'>
+<Channel Name="channel1" InputFile="./data/example.root" >
+  <Data HistoName="" InputFile="" HistoPath=""/>
+  <Sample Name="signal" HistoPath="" HistoName="signal">
+    <OverallSys Name="syst1" High="1.05" Low="0.95"/>
+    <NormFactor Name="SigXsecOverSM" Val="1" Low="0." High="3."  />
+  </Sample>
+  <Sample Name="background1" HistoPath="" NormalizeByTheory="True" HistoName="background1">
+    <StatError Activate="True" HistoName="background1_statUncert"  />
+    <OverallSys Name="syst2" Low="0.95" High="1.05"/>
+  </Sample>
+  <Sample Name="background2" HistoPath="" NormalizeByTheory="True" HistoName="background2">
+    <StatError Activate="True" /> <!-- Use Default Histogram Errors as input to StatError -->
+    <OverallSys Name="syst3" Low="0.95" High="1.05"/>
+  </Sample>
+</Channel>

From 04cb8117a0aec8e2087782104ae519ca24483b8e Mon Sep 17 00:00:00 2001
From: Matthew Feickert <matthew.feickert@cern.ch>
Date: Thu, 6 Apr 2023 02:17:10 -0500
Subject: [PATCH 5/7] Apply formatting change to NotImplementedError message

---
 src/pyhf/readxml.py  | 2 +-
 tests/test_import.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pyhf/readxml.py b/src/pyhf/readxml.py
index 55c592bb43..d1942bfe4e 100644
--- a/src/pyhf/readxml.py
+++ b/src/pyhf/readxml.py
@@ -281,7 +281,7 @@ def process_data(
 
     if inputfile == "" or histoname == "":
         raise NotImplementedError(
-            "conversion of workspaces without data is currently not supported, see issue #566"
+            "Conversion of workspaces without data is currently not supported.\nSee https://github.com/scikit-hep/pyhf/issues/566"
         )
 
     data, _ = import_root_histogram(resolver, inputfile, histopath, histoname)
diff --git a/tests/test_import.py b/tests/test_import.py
index 2cff4fa1c7..884bdd07b5 100644
--- a/tests/test_import.py
+++ b/tests/test_import.py
@@ -500,7 +500,7 @@ def test_import_noChannelDataPaths(mocker, datadir):
     with pytest.raises(NotImplementedError) as excinfo:
         pyhf.readxml.parse(basedir.joinpath("config/example.xml"), basedir)
     assert (
-        'conversion of workspaces without data is currently not supported, see issue #566'
+        "Conversion of workspaces without data is currently not supported.\nSee https://github.com/scikit-hep/pyhf/issues/566"
         in str(excinfo.value)
     )
 

From bc848f4f359604ca3bcf41ed1bc7f9d1bbb14e98 Mon Sep 17 00:00:00 2001
From: Matthew Feickert <matthew.feickert@cern.ch>
Date: Thu, 6 Apr 2023 02:24:04 -0500
Subject: [PATCH 6/7] Use pytest.raises with match

---
 tests/test_import.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tests/test_import.py b/tests/test_import.py
index 884bdd07b5..6cf2a0b8bf 100644
--- a/tests/test_import.py
+++ b/tests/test_import.py
@@ -497,12 +497,11 @@ def test_import_noChannelDataPaths(mocker, datadir):
     mocker.patch('pyhf.readxml.import_root_histogram', return_value=(_data, _err))
 
     basedir = datadir.joinpath("xmlimport_noChannelDataPaths")
-    with pytest.raises(NotImplementedError) as excinfo:
+    with pytest.raises(
+        NotImplementedError,
+        match="Conversion of workspaces without data is currently not supported.\nSee https://github.com/scikit-hep/pyhf/issues/566",
+    ):
         pyhf.readxml.parse(basedir.joinpath("config/example.xml"), basedir)
-    assert (
-        "Conversion of workspaces without data is currently not supported.\nSee https://github.com/scikit-hep/pyhf/issues/566"
-        in str(excinfo.value)
-    )
 
 
 def test_import_missingPOI(mocker, datadir):

From 34e0b7b06e9b23b4187639f18bbd1b5bf6944d5e Mon Sep 17 00:00:00 2001
From: Matthew Feickert <matthew.feickert@cern.ch>
Date: Thu, 6 Apr 2023 13:42:57 -0500
Subject: [PATCH 7/7] format

---
 tests/test_import.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_import.py b/tests/test_import.py
index 6cf2a0b8bf..ad473e144a 100644
--- a/tests/test_import.py
+++ b/tests/test_import.py
@@ -494,7 +494,7 @@ def test_import_noChannelData(mocker, datadir):
 def test_import_noChannelDataPaths(mocker, datadir):
     _data = [0.0]
     _err = [1.0]
-    mocker.patch('pyhf.readxml.import_root_histogram', return_value=(_data, _err))
+    mocker.patch("pyhf.readxml.import_root_histogram", return_value=(_data, _err))
 
     basedir = datadir.joinpath("xmlimport_noChannelDataPaths")
     with pytest.raises(