Skip to content

Commit ff854aa

Browse files
committed
Fix fill segfaults and broadcasting misbehavior (#233)
* update to latest boost.histogram * removing hot-fix * another update * fixed tests and added new tests for previous bug
1 parent a1d2a20 commit ff854aa

File tree

3 files changed

+34
-19
lines changed

3 files changed

+34
-19
lines changed

include/boost/histogram/python/register_histogram.hpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,10 +265,7 @@ register_histogram(py::module &m, const char *name, const char *desc) {
265265
// specialization for string (HD: this is very inefficient
266266
// and will be made more efficient in the future)
267267
if(py::isinstance<py::str>(x))
268-
// hot-fix, should be `v = py::cast<std::string>(x);`
269-
// once the issue in boost::histogram is fixed
270-
v = std::vector<std::string>(1,
271-
py::cast<std::string>(x));
268+
v = py::cast<std::string>(x);
272269
else if(py::isinstance<py::array>(x)) {
273270
if(py::cast<py::array>(x).ndim() != 1)
274271
throw std::invalid_argument(

tests/test_histogram.py

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,6 @@ def test_out_of_range():
369369
h[4]
370370

371371

372-
# CLASSIC: This used to have variance
373372
def test_operators():
374373
h = bh.Histogram(bh.axis.Integer(0, 2))
375374
h.fill(0)
@@ -386,7 +385,6 @@ def test_operators():
386385
h + h2
387386

388387

389-
# CLASSIC: reduce_to -> project,
390388
def test_project():
391389
h = bh.Histogram(bh.axis.Integer(0, 2), bh.axis.Integer(1, 4))
392390
h.fill(0, 1)
@@ -535,11 +533,11 @@ def test_numpy_conversion_0():
535533

536534
def test_numpy_conversion_1():
537535
# CLASSIC: was weight array
538-
a = bh.Histogram(bh.axis.Integer(0, 3))
536+
h = bh.Histogram(bh.axis.Integer(0, 3))
539537
for i in range(10):
540-
a.fill(1, weight=3)
541-
c = np.array(a) # a copy
542-
v = np.asarray(a) # a view
538+
h.fill(1, weight=3)
539+
c = np.array(h) # a copy
540+
v = np.asarray(h) # a view
543541
assert c.dtype == np.double # CLASSIC: np.float64
544542
assert_array_equal(c, np.array((0, 30, 0)))
545543
assert_array_equal(v, c)
@@ -684,8 +682,8 @@ def ar(*args):
684682
with pytest.raises(ValueError):
685683
a.fill([1, 0, 2], [1, 1])
686684

687-
# This actually broadcasts
688-
a.fill([1, 0], [1])
685+
# this broadcasts
686+
a.fill([1, 0], 1)
689687

690688
with pytest.raises(IndexError):
691689
a[1]
@@ -742,9 +740,9 @@ def ar(*args):
742740
with pytest.raises(ValueError):
743741
a.fill((1, 2), weight=([1, 1], [2, 2]))
744742

745-
# CLASSIC: Used to fail
746743
a = bh.Histogram(bh.axis.Integer(0, 3))
747-
a.fill((1, 2), weight=(1,))
744+
# this broadcasts
745+
a.fill((1, 2), weight=1)
748746
assert a[1] == 1.0
749747
assert a[2] == 1.0
750748

@@ -793,7 +791,27 @@ def test_fill_with_sequence_2():
793791
assert b[1, bh.overflow] == 1
794792

795793

796-
## this segfaults
797-
# def test_fill_with_sequence_3():
798-
# h = bh.Histogram(bh.axis.StrCategory([], growth=True), bh.axis.Integer(0, 1))
799-
# h.fill(["1"], np.arange(2))
794+
def test_fill_with_sequence_3():
795+
h = bh.Histogram(bh.axis.StrCategory([], growth=True))
796+
h.fill("A")
797+
# should use h.axes[...] once that is fixed
798+
assert h._axis(0).size == 1
799+
h.fill(["A"])
800+
assert h._axis(0).size == 1
801+
h.fill(["A", "B"])
802+
assert h._axis(0).size == 2
803+
assert_array_equal(h.view(True), [3, 1])
804+
805+
806+
def test_fill_with_sequence_4():
807+
h = bh.Histogram(
808+
bh.axis.StrCategory([], growth=True), bh.axis.Integer(0, 0, growth=True)
809+
)
810+
h.fill("1", np.arange(2))
811+
# should use h.axes[...] once that is fixed
812+
assert h._axis(0).size == 1
813+
assert h._axis(1).size == 2
814+
assert_array_equal(h.view(True), [[1, 1]])
815+
816+
with pytest.raises(ValueError):
817+
h.fill(["1"], np.arange(2)) # lengths do not match

0 commit comments

Comments
 (0)