fix: allow axis + other (#982)

* fix: allow axis + other

Signed-off-by: Henry Schreiner <[email protected]>

* fix: support unordered axes

Signed-off-by: Henry Schreiner <[email protected]>

* refactor: pull out a helper function

Signed-off-by: Henry Schreiner <[email protected]>

---------

Signed-off-by: Henry Schreiner <[email protected]>

Author: henryiii

src/boost_histogram/histogram.py

@@ -44,7 +44,7 @@
     if "_core" not in str(err):
         raise
 
-    new_msg = "Did you forget to compile boost-histogram? Use CMake or Setuptools to build, see the readme."
+    new_msg = "Did you forget to compile boost-histogram? Use CMake or scikit-build-core to build, see the readme."
 
     if sys.version_info >= (3, 11):
         err.add_note(new_msg)
@@ -178,6 +178,24 @@ def _expand_ellipsis(indexes: Iterable[Any], rank: int) -> list[Any]:
     raise IndexError("an index can only have a single ellipsis ('...')")
 
 
+def _combine_group_contents(
+    new_view: np.typing.NDArray[Any],
+    reduced_view: np.typing.NDArray[Any],
+    i: int,
+    j: int,
+    jj: int,
+) -> None:
+    """
+    Combine two views into one, in-place. This is used for threaded filling.
+    """
+    pos = [slice(None)] * (i)
+    if new_view.dtype.names:
+        for field in new_view.dtype.names:
+            new_view[(*pos, jj, ...)][field] += reduced_view[(*pos, j, ...)][field]
+    else:
+        new_view[(*pos, jj, ...)] += reduced_view[(*pos, j, ...)]
+
+
 H = TypeVar("H", bound="Histogram")
 
 
@@ -992,18 +1010,18 @@ def __getitem__(self: H, index: IndexingExpr) -> H | float | Accumulator:
 
                     for new_j, group in enumerate(groups):
                         for _ in range(group):
-                            pos = [slice(None)] * (i)
-                            if new_view.dtype.names:
-                                for field in new_view.dtype.names:
-                                    new_view[(*pos, new_j + new_j_base, ...)][
-                                        field
-                                    ] += reduced_view[(*pos, j, ...)][field]
-                            else:
-                                new_view[(*pos, new_j + new_j_base, ...)] += (
-                                    reduced_view[(*pos, j, ...)]
-                                )
+                            _combine_group_contents(
+                                new_view, reduced_view, i, j, new_j + new_j_base
+                            )
                             j += 1
 
+                        if (
+                            old_axis.traits_underflow
+                            and not axes[i].traits_ordered
+                            and axes[i].traits_overflow
+                        ):
+                            _combine_group_contents(new_view, reduced_view, i, 0, -1)
+
                     reduced = new_reduced
 
         # Will be updated below

src/boost_histogram/tag.py

@@ -130,19 +130,19 @@ def __init__(
         edges: Sequence[int | float] | None = None,
         axis: PlottableAxis | None = None,
     ) -> None:
-        if (
-            sum(i is not None for i in [factor_or_axis, factor, groups, edges, axis])
-            != 1
-        ):
+        if isinstance(factor_or_axis, int):
+            factor = factor_or_axis
+        elif factor_or_axis is not None:
+            axis = factor_or_axis
+
+        total_args = sum(i is not None for i in [factor, groups, edges])
+        if total_args != 1 and axis is None:
             raise ValueError("Exactly one argument should be provided")
+
         self.groups = groups
         self.edges = edges
         self.axis = axis
         self.factor = factor
-        if isinstance(factor_or_axis, int):
-            self.factor = factor_or_axis
-        elif factor_or_axis is not None:
-            self.axis = factor_or_axis
 
     def __repr__(self) -> str:
         repr_str = f"{self.__class__.__name__}"
@@ -177,10 +177,10 @@ def group_mapping(self, axis: PlottableAxis) -> Sequence[int]:
             return [self.factor] * len(axis)
         if self.edges is not None or self.axis is not None:
             newedges = None
-            if self.axis is not None and hasattr(self.axis, "edges"):
-                newedges = self.axis.edges
-            elif self.edges is not None:
+            if self.edges is not None:
                 newedges = self.edges
+            elif self.axis is not None and hasattr(self.axis, "edges"):
+                newedges = self.axis.edges
 
             if newedges is not None and hasattr(axis, "edges"):
                 assert newedges[0] == axis.edges[0], "Edges must start at first bin"

tests/test_histogram.py

@@ -688,6 +688,24 @@ def test_rebin_1d_flow():
     assert_array_equal(hs.view(flow=True), [2, 2])
 
 
+def test_rebin_change_axis_int():
+    h = bh.Histogram(bh.axis.Regular(5, 0, 5))
+    h.fill([-1, 1.1, 2.2, 3.3, 4.4, 5.5])
+    hs = h[bh.rebin(edges=[0, 3, 5.0], axis=bh.axis.Integer(10, 12))]
+    assert_array_equal(hs.view(), [2, 2])
+    assert_array_equal(hs.view(flow=True), [1, 2, 2, 1])
+    assert_array_equal(hs.axes.edges[0], [10, 11, 12])
+
+
+def test_rebin_change_axis_cat():
+    h = bh.Histogram(bh.axis.Regular(5, 0, 5))
+    h.fill([-1, 1.1, 2.2, 3.3, 4.4, 5.5])
+    hs = h[bh.rebin(groups=[2, 3], axis=bh.axis.StrCategory(["a", "b"]))]
+    assert_array_equal(hs.view(), [1, 3])
+    assert_array_equal(hs.view(flow=True), [1, 3, 4])
+    assert_array_equal(list(hs.axes[0]), ["a", "b"])
+
+
 def test_shrink_rebin_1d():
     h = bh.Histogram(bh.axis.Regular(20, 0, 4))
     h.fill(1.1)