Accept abstract unit ranges as count levels by Sinkleberg · Pull Request #1021 · JuliaStats/StatsBase.jl

Sinkleberg · 2026-11-05T19:07:47Z

Fixes #908.

counts([1, 1, 2, 3], Base.OneTo(4)) now returns [2, 1, 1, 0]. Weighted counts and proportions use the same bin order.

I used Codex for the patch and tests. The StatsBase suite passed on Julia 1.10.13.

andreasnoack · 2026-11-10T08:51:27Z

The signature in the docs would have to be adjusted as well. That's why the docs build fails.

devmotion · 2026-11-10T09:39:36Z

 Add the number of occurrences in `x` of each value in `levels` to an existing
-array `r`. For each `xi ∈ x`, if `xi == levels[j]`, then we increment `r[j]`.
+array `r`. The `j`th bin counts the `j`th value in `levels`, independently of the
+indices of `levels`.


Nit: maybe make the link to r explicit?

Suggested change

Add the number of occurrences in `x` of each value in `levels` to an existing

array `r`. For each `xi ∈ x`, if `xi == levels[j]`, then we increment `r[j]`.

array `r`. The `j`th bin counts the `j`th value in `levels`, independently of the

indices of `levels`.

Add the number of occurrences in `x` of each value in `levels` to an existing

array `r`. `r[j]` counts the `j`th value in `levels`, independently of the

indices of `levels`.

devmotion · 2026-11-10T09:39:36Z

    # add counts of integers from x that fall within levels to r

-    checkbounds(r, axes(levels)...)
+    checkbounds(r, Base.OneTo(length(levels)))


This is correct (a single index checks the linear indices 1:length(levels) of r, exactly the indices written below), but IMO 1:length(levels) reads more clearly:

Suggested change

checkbounds(r, Base.OneTo(length(levels)))

checkbounds(r, 1:length(levels))

devmotion · 2026-11-10T09:39:37Z

+    # Promote narrow bounds before subtracting; keep generic integer arithmetic.
+    lower = m0 + zero(Int)

    for xi in x
        if m0 <= xi <= m1
-            r[xi - b] += 1
+            r[(xi - lower) + 1] += 1
        end
    end


Since m0 <= xi <= m1, the offset xi - m0 lies in 0:(length(levels) - 1) and fits in an Int. So we can compute it modulo 2^N and avoid both the wrap-around of m0 - 1 and the overflow of xi - m0 for narrow types, without relying on mixed-type promotion. The index is then always an Int.

In a quick benchmark it gives identical results to this PR on all the edge cases in the new tests (and a few more), with the same speed for Int/Int8 input and ~20x faster for BigInt input (no BigInt index allocated per element).

Caveat: for custom Integer types without a specialized rem(::T, ::Type{UInt}), Base falls back to convert, which throws for negative values.

Suggested change

# Promote narrow bounds before subtracting; keep generic integer arithmetic.

lower = m0 + zero(Int)

for xi in x

if m0 <= xi <= m1

r[xi - b] += 1

r[(xi - lower) + 1] += 1

end

end

# `m0 <= xi <= m1` implies `0 <= xi - m0 < length(levels) <= length(r) <= typemax(Int)`

# (by the `checkbounds` call above), so the offset is computed exactly in modular

# `UInt` arithmetic: no overflow for small integer types and no allocations for `BigInt`.

u0 = m0 % UInt

for xi in x

if m0 <= xi <= m1

r[((xi % UInt) - u0) % Int + 1] += 1

end

end

devmotion · 2026-11-10T09:39:37Z

+    checkbounds(r, Base.OneTo(length(levels)))

    m0 = first(levels)
    m1 = last(levels)
-    b = m0 - 1
+    lower = m0 + zero(Int)

    for i in eachindex(xv, wv)
        xi = xv[i]
        if m0 <= xi <= m1
-            r[xi - b] += wv[i]
+            r[(xi - lower) + 1] += wv[i]
        end
    end


Same here:

Suggested change

checkbounds(r, Base.OneTo(length(levels)))

m0 = first(levels)

m1 = last(levels)

b = m0 - 1

lower = m0 + zero(Int)

for i in eachindex(xv, wv)

xi = xv[i]

if m0 <= xi <= m1

r[xi - b] += wv[i]

r[(xi - lower) + 1] += wv[i]

end

end

checkbounds(r, 1:length(levels))

m0 = first(levels)

m1 = last(levels)

# See the unweighted method: the offset `xi - m0` is exact in modular `UInt` arithmetic.

u0 = m0 % UInt

for i in eachindex(xv, wv)

xi = xv[i]

if m0 <= xi <= m1

r[((xi % UInt) - u0) % Int + 1] += wv[i]

end

end

devmotion · 2026-11-10T09:39:37Z

+    @test addcounts!(ones(Int, 4), x, levels) == [2, 2, 1, 1]
+    @test counts(x, levels, w) == [3.0, 4.0, 0.0, 0.0]
+    @test proportions(x, levels) == [0.25, 0.25, 0.0, 0.0]
+    @test proportions(x, levels, w) ≈ [0.3, 0.4, 0.0, 0.0]


Could you add tests for the checkbounds error path, for weighted addcounts! with a non-zero r, and for the exact examples from #908?

Suggested change

@test proportions(x, levels, w) ≈ [0.3, 0.4, 0.0, 0.0]

@test proportions(x, levels, w) ≈ [0.3, 0.4, 0.0, 0.0]

@test addcounts!(ones(4), x, levels, w) == [4.0, 5.0, 1.0, 1.0]

# `r` must have linear indices `1:length(levels)`

@test_throws BoundsError addcounts!(zeros(Int, 3), x, levels)

@test_throws BoundsError addcounts!(zeros(3), x, levels, w)

@test_throws BoundsError addcounts!(OffsetVector(zeros(Int, 4), 2:5), x, levels)

# Examples from #908

@test counts(x, Base.OneTo(4), w) == [3.0, 3.0, 4.0, 0.0]

@test proportions(x, Base.OneTo(4)) == [0.5, 0.25, 0.25, 0.0]

@test proportions(x, Base.OneTo(4), w) ≈ [0.3, 0.3, 0.4, 0.0]

devmotion · 2026-11-10T09:39:37Z

+end
+
+@testset "integer count bin arithmetic" begin
+    # The 201st bin must not wrap during Int8 subtraction.


Most of this testset already passes on master: 28 of its 32 tests pass there. In particular the Int8 case worked before, since m0 - 1 promotes to Int. The actual regression is first(levels) - 1 wrapping when x has a wider eltype, e.g. counts(BigInt[0, 1], UInt(0):UInt(1)) throws InexactError on master. Could you reduce this testset to those regressions (BigInt input with UInt64/UInt128 ranges starting at 0, BigInt input with a range starting at typemin(Int)) plus a single small-integer case guarding against overflow in xi - first(levels)?

devmotion · 2026-11-10T09:39:37Z

+    @test x == Int8[-101, -100, 100, 101]
+    @test w == weights([1.0, 2.0, 3.0, 4.0])


These non-mutation checks are unrelated to this change, so I'd remove them.

devmotion · 2026-11-10T09:39:37Z

+    @test x == Int8[-101, -100, 100, 101]
+    @test w == weights([1.0, 2.0, 3.0, 4.0])
+
+    # Bool is an Integer but does not have a widen method.


This comment is outdated: the implementation does not use widen.

devmotion · 2026-11-10T09:39:37Z

+
+    # Use ordinal bins with mixed signedness and abstract eltypes.
+    mixed = Integer[Int8(-100), UInt64(100), big(0)]
+    expected[101] = 1


Changing expected halfway through the testset makes the following tests hard to follow. Could you use a separate expected vector?

devmotion · 2026-11-10T09:39:37Z

+    @test counts(unsigned, levels, weights([2.0, 3.0, 4.0])) ==
+        [i == 101 ? 2.0 : i == 201 ? 3.0 : 0.0 for i in 1:201]
+
+    # Convert both the range bound and input before the index arithmetic.


This comment is inaccurate: the input is not converted.

Accept abstract unit ranges as count levels

5eb13ee

Match count documentation to abstract unit range signature

c1f42ce

devmotion reviewed Nov 10, 2026

View reviewed changes

Preserve generic integer counting with the modular index fast path

ffe67af

Navigation Menu

Repository navigation

Accept abstract unit ranges as count levels - #1021

Accept abstract unit ranges as count levels#1021
Sinkleberg wants to merge 3 commits into
JuliaStats:masterfrom
Sinkleberg:codex/count-abstract-unit-ranges

Sinkleberg commented Nov 5, 2026

Uh oh!

andreasnoack commented Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

devmotion Nov 10, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	checkbounds(r, Base.OneTo(length(levels)))
	checkbounds(r, 1:length(levels))

-    @test proportions(x, levels, w) ≈ [0.3, 0.4, 0.0, 0.0]
+    @test proportions(x, levels, w) ≈ [0.3, 0.4, 0.0, 0.0]
+    @test addcounts!(ones(4), x, levels, w) == [4.0, 5.0, 1.0, 1.0]
+    # `r` must have linear indices `1:length(levels)`
+    @test_throws BoundsError addcounts!(zeros(Int, 3), x, levels)
+    @test_throws BoundsError addcounts!(zeros(3), x, levels, w)
+    @test_throws BoundsError addcounts!(OffsetVector(zeros(Int, 4), 2:5), x, levels)
+    # Examples from #908
+    @test counts(x, Base.OneTo(4), w) == [3.0, 3.0, 4.0, 0.0]
+    @test proportions(x, Base.OneTo(4)) == [0.5, 0.25, 0.25, 0.0]
+    @test proportions(x, Base.OneTo(4), w) ≈ [0.3, 0.3, 0.4, 0.0]

		@test x == Int8[-101, -100, 100, 101]
		@test w == weights([1.0, 2.0, 3.0, 4.0])

Conversation

Sinkleberg commented Nov 5, 2026

Uh oh!

andreasnoack commented Nov 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants