From 53361dd2f75598ff12ef31a8da4c19574954d572 Mon Sep 17 00:00:00 2001
From: Matthew Feickert <matthew.feickert@cern.ch>
Date: Thu, 6 Apr 2023 16:18:19 -0500
Subject: [PATCH] fix(backport): Raise error when converting xml workspaces
 without data (#2166)

* Backport PR https://github.com/scikit-hep/pyhf/pull/2109
---
 src/pyhf/readxml.py                           |   5 +
 tests/test_import.py                          |  13 ++
 .../config/HistFactorySchema.dtd              | 160 ++++++++++++++++++
 .../config/example.xml                        |   8 +
 .../config/example_channel.xml                |  16 ++
 5 files changed, 202 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/src/pyhf/readxml.py b/src/pyhf/readxml.py
index 165d51685c..d1942bfe4e 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 histoname == "":
+        raise NotImplementedError(
+            "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)
     return data
 
diff --git a/tests/test_import.py b/tests/test_import.py
index 78f7b27ba1..7e1f52895b 100644
--- a/tests/test_import.py
+++ b/tests/test_import.py
@@ -491,6 +491,19 @@ 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,
+        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)
+
+
 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>