refactor: boolean axis with reduce and int fill (#422)

* refactor: use custom boolean, faster compile, simpler fill

* fix: restore boolean behavior, slighly faster too

* refactor: drop boolean workaround, allow slicing

Author: henryiii

include/bh_python/axis.hpp

@@ -108,7 +108,27 @@ using category_str_growth
 BHP_SPECIALIZE_NAME(category_str)
 BHP_SPECIALIZE_NAME(category_str_growth)
 
-using boolean = bh::axis::boolean<metadata_t>;
+class boolean : public bh::axis::integer<int, metadata_t, option::none_t> {
+  public:
+    explicit boolean(metadata_t meta = {})
+        : integer(0, 2, std::move(meta)) {}
+    explicit boolean(const boolean& src,
+                     bh::axis::index_type begin,
+                     bh::axis::index_type end,
+                     unsigned merge)
+        : integer(src, begin, end, merge) {}
+    boolean(const boolean& other)
+        : integer(other) {}
+
+    bh::axis::index_type index(int x) const noexcept {
+        return integer::index(x == 0 ? 0 : 1);
+    }
+
+    // We can't specify inclusive, since this could be sliced
+};
+
+// Built-in boolean requires bool fill, slower compile, not reducible
+// using boolean = bh::axis::boolean<metadata_t>;
 BHP_SPECIALIZE_NAME(boolean)
 
 // Axis defined elsewhere
@@ -154,6 +174,11 @@ select(Continuous&&, Discrete&&, Integer&& i, const bh::axis::integer<int, Ts...
     return std::forward<Integer>(i)(ax);
 }
 
+template <class Continuous, class Discrete, class Boolean, class... Ts>
+decltype(auto) select(Continuous&&, Discrete&&, Boolean&& i, const boolean& ax) {
+    return std::forward<Boolean>(i)(ax);
+}
+
 /// Return bin center for continuous axis and bin value for discrete axis
 template <class A>
 double unchecked_center(const A& ax, bh::axis::index_type i) {

include/bh_python/fill.hpp

@@ -32,8 +32,8 @@ namespace variant = boost::variant2;
 static_assert(
     mp11::mp_empty<mp11::mp_set_difference<
         mp11::mp_unique<mp11::mp_transform<bh::axis::traits::value_type, axis_variant>>,
-        mp11::mp_list<double, int, std::string, bool>>>::value,
-    "supported value types are double, int, std::string, bool; "
+        mp11::mp_list<double, int, std::string>>>::value,
+    "supported value types are double, int, std::string; "
     "new axis was added with different value type");
 
 template <class T>
@@ -60,7 +60,7 @@ struct c_array_t<std::string> : std::vector<std::string> {
     }
 };
 
-// for int, double, bool
+// for int, double
 template <class T>
 bool is_value(py::handle h) {
     if(py::isinstance<py::array>(h) && py::cast<py::array>(h).ndim() > 0)
@@ -99,8 +99,6 @@ using arg_t = variant::variant<c_array_t<double>,
                                double,
                                c_array_t<int>,
                                int,
-                               c_array_t<bool>,
-                               bool,
                                c_array_t<std::string>,
                                std::string>;
 
@@ -116,7 +114,7 @@ inline auto get_vargs(const vector_axis_variant& axes, const py::args& args) {
         axes,
         [args_it = args.begin(), vargs_it = vargs.begin()](const auto& ax) mutable {
             using T = bh::axis::traits::value_type<std::decay_t<decltype(ax)>>;
-            // T is one of: int, double, std::string, bool
+            // T is one of: int, double, std::string
 
             const auto& x = *args_it++;
             auto& v       = *vargs_it++;

src/boost_histogram/_internal/axis.py

@@ -616,7 +616,17 @@ def __init__(self, **kwargs):
 
     def _repr_args(self):
         "Return inner part of signature for use in repr"
-        return ""
+        if self.size == 2:
+            return ""
+        elif self.size == 0:
+            return "<empty>"
+        elif self.size == 1 and self.centers[0] < 0.75:
+            return "<False>"
+        elif self.size == 1:
+            return "<True>"
+        else:
+            # Shouldn't be possible, can't grow
+            "<unknown>"
 
     def _repr_kwargs(self):
         """

src/boost_histogram/_internal/hist.py

@@ -552,9 +552,6 @@ def __getitem__(self, index):  # noqa: C901
 
         indexes = self._compute_commonindex(index)
 
-        # Workaround for missing "pick" and no reduction on bool axes
-        pick_bool = {}
-
         # If this is (now) all integers, return the bin contents
         # But don't try *dict!
         if not hasattr(indexes, "items"):
@@ -569,11 +566,7 @@ def __getitem__(self, index):  # noqa: C901
         # Compute needed slices and projections
         for i, ind in enumerate(indexes):
             if hasattr(ind, "__index__"):
-                if isinstance(self.axes[i]._ax, _core.axis.boolean):
-                    pick_bool[i] = ind
-                    ind = slice(None, None, sum)
-                else:
-                    ind = slice(ind.__index__(), ind.__index__() + 1, sum)
+                ind = slice(ind.__index__(), ind.__index__() + 1, sum)
 
             elif not isinstance(ind, slice):
                 raise IndexError(
@@ -614,13 +607,6 @@ def __getitem__(self, index):  # noqa: C901
 
                 slices.append(_core.algorithm.slice_and_rebin(i, start, stop, merge))
 
-        for i, select in pick_bool.items():
-            self = self.copy()
-            view = self.view(flow=True)
-            view[
-                tuple(~select if j == i else slice(None) for j in range(self.ndim))
-            ] = 0
-
         reduced = self._hist.reduce(*slices)
 
         if not integrations:

tests/test_histogram.py

@@ -667,16 +667,36 @@ def test_pickle_1():
     assert a == b
 
 
+def test_fill_bool_not_bool():
+    h = bh.Histogram(bh.axis.Boolean())
+
+    h.fill([0, 1, 1, 7, -3])
+
+    assert_array_equal(h.view(), [1, 4])
+
+
 def test_pick_bool():
-    a = bh.Histogram(bh.axis.Boolean(), bh.axis.Boolean(metadata={"one": 1}))
+    h = bh.Histogram(bh.axis.Boolean(), bh.axis.Boolean(metadata={"one": 1}))
 
-    a.fill([True, True, False, False], [True, False, True, True])
-    a.fill([True, True, True], True)
+    h.fill([True, True, False, False], [True, False, True, True])
+    h.fill([True, True, True], True)
+
+    assert_array_equal(h[True, :].view(), [1, 4])
+    assert_array_equal(h[False, :].view(), [0, 2])
+    assert_array_equal(h[:, False].view(), [0, 1])
+    assert_array_equal(h[:, True].view(), [2, 4])
+
+
+def test_slice_bool():
+    h = bh.Histogram(bh.axis.Boolean())
+    h.fill([0, 0, 0, 1, 3, 4, -2])
+
+    assert_array_equal(h.view(), [3, 4])
+    assert_array_equal(h[1:].view(), [4])
+    assert_array_equal(h[:1].view(), [3])
 
-    assert_array_equal(a[True, :].view(), [1, 4])
-    assert_array_equal(a[False, :].view(), [0, 2])
-    assert_array_equal(a[:, False].view(), [0, 1])
-    assert_array_equal(a[:, True].view(), [2, 4])
+    assert_array_equal(h[:1].axes[0].centers, [0.5])
+    assert_array_equal(h[1:].axes[0].centers, [1.5])
 
 
 def test_pickle_bool():