Repository: scikit-hep/hist

Issue:
[BUG] Error message when category slicing info isn't present in the axies
**Describe the bug**

If you create a category axis, fill, and then slice it using the list operator when you ask for a category that doesn't exist the error message is rather hard to interpret. Should be improved.

**Steps to reproduce**

This [notebook](https://github.com/iris-hep/opendata-higgs-discovery/blob/main/notebooks/hist-demo-cat-clice-errors.ipynb) shows how this works with `hist` version 2.4.0. A error message might be more like "x, y, and z, are valid in this category axis" or similar.
[BUG] Uncertainty bands in efficiency ratios go above unity
**Describe the bug**

The error bars produced in the ratio of an efficiency style plot (`hist.plot_ratio(..., rp_uncertainty_type = "poisson-ratio", ...)`)
can extend above unity. This is unexpected and not meaningful if the numerator is a true subset of the denominator.

**Steps to reproduce**

This is observed using `hist==2.4.0`.

Following the example from the [docs](https://hist.readthedocs.io/en/latest/user-guide/notebooks/Plots.html),

```python
hist0 = hist.Hist(hist.axis.Regular(50, -5, 5), underflow = False, overflow = False).fill(np.random.normal(size=1700))
hist1 = hist0.copy() * 0.98

fig = plt.figure(figsize = (10,8))
_ = hist1.plot_ratio(
    hist0,
    rp_ylabel="Efficiency",
    rp_num_label="hist1",
    rp_denom_label="hist0",
    rp_uncert_draw_type="line",
    rp_uncertainty_type="poisson-ratio",
    rp_ylim=[0.9,1.1],
)
```
This produces something like the following,
<img width="863" alt="image" src="https://user-images.githubusercontent.com/7841565/128089782-f1bd705c-50d2-443f-8daa-5375bf853cab.png">

where we see the error bars going above unity.

If I take the same data used in `hist0` and `hist1` above, and produce a `TEfficiency` object in `ROOT`, I get something like the following,
<img width="615" alt="image" src="https://user-images.githubusercontent.com/7841565/128089882-973ec586-83be-4891-9b44-332830c0ac6c.png">
which is something more like what I would expect since I believe `hist` is using a Clopper-Pearson interval (default in ROOT).






Discussion:

Patch:
diff --git a/docs/changelog.rst b/docs/changelog.rst
index ea8b25d..a2f849e 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -7,8 +7,9 @@ Version 2.5.0
 * Dropped Python 3.6 support.
   `#194 <https://github.com/scikit-hep/hist/pull/194>`_
 
-* Add ``"efficiency"`` ``uncertainty_type`` option alias for ``ratio_plot`` API.
+* Add ``"efficiency"`` ``uncertainty_type`` option for ``ratio_plot`` API.
   `#266 <https://github.com/scikit-hep/hist/pull/266>`_
+  `#278 <https://github.com/scikit-hep/hist/pull/278>`_
 
 
 Version 2.4.0
diff --git a/docs/user-guide/notebooks/Plots.ipynb b/docs/user-guide/notebooks/Plots.ipynb
index 87f0dc2..cbc5bba 100644
--- a/docs/user-guide/notebooks/Plots.ipynb
+++ b/docs/user-guide/notebooks/Plots.ipynb
@@ -310,12 +310,10 @@
     "fig = plt.figure(figsize=(10, 8))\n",
     "main_ax_artists, sublot_ax_arists = hist_3.plot_ratio(\n",
     "    hist_2,\n",
-    "    rp_ylabel=\"Efficiency\",\n",
     "    rp_num_label=\"hist3\",\n",
     "    rp_denom_label=\"hist2\",\n",
     "    rp_uncert_draw_type=\"line\",\n",
-    "    rp_uncertainty_type=\"poisson-ratio\",  # or \"efficiency\"\n",
-    "    rp_ylim=[0, 1.1],\n",
+    "    rp_uncertainty_type=\"efficiency\",\n",
     ")"
    ]
   }
diff --git a/src/hist/intervals.py b/src/hist/intervals.py
index 826db7c..6d9de11 100644
--- a/src/hist/intervals.py
+++ b/src/hist/intervals.py
@@ -127,8 +127,11 @@ def ratio_uncertainty(
          ``"poisson"`` (default) implements the Poisson interval for the
          numerator scaled by the denominator.
          ``"poisson-ratio"`` implements the Clopper-Pearson interval for Poisson
-         distributed ``num`` and ``denom``.
-         ``"efficiency"`` is an alias for ``"poisson-ratio"``.
+         distributed ``num`` and ``denom`` where it is assumed that ``num`` and
+         ``denom`` are independent.
+         ``"efficiency"`` implements the Clopper-Pearson interval for Poisson
+         distributed ``num`` and ``denom``, with ``num`` assumed to be a
+         strict subset of ``denom``.
 
     Returns:
         The uncertainties for the ratio.
@@ -138,9 +141,11 @@ def ratio_uncertainty(
         ratio = num / denom
     if uncertainty_type == "poisson":
         ratio_uncert = np.abs(poisson_interval(ratio, num / np.square(denom)) - ratio)
-    elif uncertainty_type in {"poisson-ratio", "efficiency"}:
+    elif uncertainty_type == "poisson-ratio":
         # poisson ratio n/m is equivalent to binomial n/(n+m)
         ratio_uncert = np.abs(clopper_pearson_interval(num, num + denom) - ratio)
+    elif uncertainty_type == "efficiency":
+        ratio_uncert = np.abs(clopper_pearson_interval(num, denom) - ratio)
     else:
         raise TypeError(
             f"'{uncertainty_type}' is an invalid option for uncertainty_type."
diff --git a/src/hist/plot.py b/src/hist/plot.py
index 211215c..0619eea 100644
--- a/src/hist/plot.py
+++ b/src/hist/plot.py
@@ -574,6 +574,9 @@ def _plot_ratiolike(
     rp_kwargs.setdefault("legend_loc", "best")
     rp_kwargs.setdefault("num_label", None)
     rp_kwargs.setdefault("denom_label", None)
+    if rp_kwargs["uncertainty_type"] == "efficiency":
+        rp_kwargs.setdefault("ylabel", "Efficiency")
+        rp_kwargs.setdefault("ylim", [0, 1.1])
 
     # patch plot keyword arguments
     pp_kwargs = _filter_dict(kwargs, "pp_")


Test patch:
diff --git a/tests/test_intervals.py b/tests/test_intervals.py
index be49492..0f06dc5 100644
--- a/tests/test_intervals.py
+++ b/tests/test_intervals.py
@@ -141,3 +141,22 @@ def test_ratio_uncertainty(hist_fixture):
         intervals.ratio_uncertainty(
             hist_1.values(), hist_2.values(), uncertainty_type="fail"
         )
+
+
+def test_valid_efficiency_ratio_uncertainty(hist_fixture):
+    """
+    Test that the upper bound for the error interval does not exceed unity
+    for efficiency ratio plots.
+    """
+
+    hist_1, _ = hist_fixture
+    num = hist_1.values()
+    den = num
+
+    efficiency_ratio = num / den
+    _, uncertainty_max = intervals.ratio_uncertainty(
+        num, den, uncertainty_type="efficiency"
+    )
+    efficiency_err_up = efficiency_ratio + uncertainty_max
+
+    assert len(efficiency_err_up[efficiency_err_up > 1.0]) == 0