Merge pull request #106 from ManuelHu/histogram-fixes

histogram: restrict more error messages to when they are necessary

Author: gipert

src/lgdo/lh5/_serializers/write/composite.py

@@ -70,8 +70,12 @@ def _h5_write_lgdo(
 
     # struct, table, waveform table or histogram.
     if isinstance(obj, types.Struct):
-        if isinstance(obj, types.Histogram) and wo_mode not in ["w", "o", "of"]:
-            msg = f"can't append-write histogram in wo_mode '{wo_mode}'"
+        if (
+            isinstance(obj, types.Histogram)
+            and wo_mode not in ["w", "o", "of"]
+            and name in group
+        ):
+            msg = f"can't append-write to histogram in wo_mode '{wo_mode}'"
             raise LH5EncodeError(msg, lh5_file, group, name)
         if isinstance(obj, types.Histogram) and write_start != 0:
             msg = f"can't write histogram in wo_mode '{wo_mode}' with write_start != 0"

src/lgdo/types/histogram.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import logging
 from collections.abc import Iterable
 from typing import Any
 
@@ -12,6 +13,8 @@
 from .scalar import Scalar
 from .struct import Struct
 
+log = logging.getLogger(__name__)
+
 
 class Histogram(Struct):
     class Axis(Struct):
@@ -197,6 +200,7 @@ def __init__(
         isdensity: bool = False,
         attrs: dict[str, Any] | None = None,
         binedge_attrs: dict[str, Any] | None = None,
+        flow: bool = True,
     ) -> None:
         """A special struct to contain histogrammed data.
 
@@ -221,6 +225,16 @@ def __init__(
             as binning.
         attrs
             a set of user attributes to be carried along with this LGDO.
+        flow
+            If ``False``, discard counts in over-/underflow bins of the passed
+            :class:`hist.Hist` instance. If ``True``, this data will also be discarded,
+            but a warning is emitted.
+
+            .. note ::
+
+                :class:`Histogram` does not support storing counts in overflow or
+                underflow bins. This parameter just controls, whether a warning will
+                be emitted.
         """
         if isinstance(weights, hist.Hist):
             if binning is not None:
@@ -230,9 +244,10 @@ def __init__(
                 msg = "not allowed to pass isdensity=True if constructing from hist.Hist instance"
                 raise ValueError(msg)
 
-            if weights.sum(flow=True) != weights.sum(flow=False):
-                msg = "flow bins of hist.Hist cannot be represented"
-                raise ValueError(msg)
+            if weights.sum(flow=True) != weights.sum(flow=False) and flow:
+                log.warning(
+                    "flow bins of hist.Hist cannot be represented, their counts are discarded"
+                )
             weights_view = weights.view(flow=False)
             if type(weights_view) is not np.ndarray:
                 msg = "only simple numpy-backed storages can be used in a hist.Hist"

tests/lh5/test_lh5_write.py

@@ -414,13 +414,23 @@ def test_write_histogram(caplog, tmptestdir):
 
     # Same field name, different values
     store = lh5.LH5Store()
+    # "appending" to a non-existing histogram should work.
     store.write(
         h1,
         "my_histogram",
         f"{tmptestdir}/write_histogram_test.lh5",
         group="my_group",
-        wo_mode="write_safe",
+        wo_mode="append",
     )
+    with pytest.raises(lh5.exceptions.LH5EncodeError):
+        # appending to an existing histogram should not work.
+        store.write(
+            h1,
+            "my_histogram",
+            f"{tmptestdir}/write_histogram_test.lh5",
+            group="my_group",
+            wo_mode="append",
+        )
     store.write(
         h2,
         "my_histogram",

tests/types/test_histogram.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import logging
+
 import hist
 import numpy as np
 import pytest
@@ -8,7 +10,9 @@
 from lgdo.lh5.exceptions import LH5DecodeError
 
 
-def test_init_hist_regular():
+def test_init_hist_regular(caplog):
+    caplog.set_level(logging.WARNING)
+
     h = hist.Hist(
         hist.axis.Regular(bins=10, start=0, stop=1, name="x"),
         hist.axis.Regular(bins=10, start=0, stop=1, name="y"),
@@ -34,8 +38,10 @@ def test_init_hist_regular():
         Histogram(h, binning=(np.array([0, 1, 2]),))
 
     h.fill([-1, 0.8, 2])
-    with pytest.raises(ValueError, match="flow bins"):
-        Histogram(h)
+    caplog.clear()
+    Histogram(h)
+    assert "flow bins" in caplog.text
+    caplog.clear()
 
     # assert that the hist data is not copied into the LGDO.
     h = hist.Hist(hist.axis.Regular(bins=10, start=0, stop=10))