Skip to content

out of bounds error on empty iterator indexing #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Apr 3, 2025
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
ChunkSplitters.jl Changelog
=========================
===========================

Version 3.1.2
-------------
- ![BUGFIX][badge-bugfix] Indexing chunk iterators returns out-of-bounds error if the iterator is empty now (previously returned an empty range).

Version 3.1.1
-------------
Expand All @@ -9,7 +13,6 @@ Version 3.1.0
-------------
- ![INFO][badge-info] Recover support for Julia 1.6+. All tests pass from 1.9+, and an allocation test fails in 1.6.


Version 3.0.0
-------------
- ![BREAKING][badge-breaking] The new lower compat bound for Julia is 1.10. This implies that support for the old LTS 1.6 has been dropped.
Expand Down
6 changes: 4 additions & 2 deletions src/api.jl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ The keyword arguments `n` and `size` are mutually exclusive.
* `minsize` can be used to specify the minimum size of a chunk,
and can be used in combination with the `n` keyword. If, for the given `n`, the chunks
are smaller than `minsize`, the number of chunks will be decreased to ensure that
each chunk is at least `minsize` long.
each chunk is at least `minsize` long. By default, `minsize == 1` when the keyword
is set to `nothing`.

### Noteworthy

Expand Down Expand Up @@ -95,7 +96,8 @@ The keyword arguments `n` and `size` are mutually exclusive.
* `minsize` can be used to specify the minimum size of a chunk,
and can be used in combination with the `n` keyword. If, for the given `n`, the chunks
are smaller than `minsize`, the number of chunks will be decreased to ensure that
each chunk is at least `minsize` long.
each chunk is at least `minsize` long. By default, `minsize == 1` when the keyword
is set to `nothing`.

### Noteworthy

Expand Down
36 changes: 16 additions & 20 deletions src/internals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ function err_not_chunkable(::T) where {T}
end

Base.firstindex(::AbstractChunks) = 1

Base.lastindex(c::AbstractChunks) = length(c)

Base.length(c::AbstractChunks{T,FixedCount,S}) where {T,S} = c.n
Expand All @@ -103,8 +102,8 @@ Base.getindex(c::ViewChunks{T,C,S}, i::Int) where {T,C,S} = @view(c.collection[g

Base.eltype(::IndexChunks{T,C,Consecutive}) where {T,C} = UnitRange{Int}
Base.eltype(::IndexChunks{T,C,RoundRobin}) where {T,C} = StepRange{Int,Int}
Base.eltype(c::ViewChunks{T,C,Consecutive}) where {T,C} = typeof(c[firstindex(c)])
Base.eltype(c::ViewChunks{T,C,RoundRobin}) where {T,C} = typeof(c[firstindex(c)])
Base.eltype(c::ViewChunks{T,C,Consecutive}) where {T,C} = typeof(@view(c.collection[firstindex(c.collection):lastindex(c.collection)]))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are important to allow eltype to return the proper types without having to call getchunkindices. firstindex and lastindex are considered part of the required interface.

Base.eltype(c::ViewChunks{T,C,RoundRobin}) where {T,C} = typeof(@view(c.collection[firstindex(c.collection):1:lastindex(c.collection)]))

function Base.iterate(c::AbstractChunks, state=firstindex(c))
if state > lastindex(c)
Expand Down Expand Up @@ -145,30 +144,27 @@ Base.length(ec::Enumerate{<:AbstractChunks}) = length(ec.itr)

Base.eachindex(ec::Enumerate{<:AbstractChunks}) = Base.OneTo(length(ec.itr))

_empty_itr(::Type{Consecutive}) = 0:-1
_empty_itr(::Type{RoundRobin}) = 0:1:-1

"""
getchunkindices(c::AbstractChunks, i::Integer)

Returns the range of indices of `collection` that corresponds to the `i`-th chunk.
"""
function getchunkindices(c::AbstractChunks{T,C,S}, ichunk::Integer) where {T,C,S}
length(c) == 0 && return _empty_itr(S)
ichunk <= length(c.collection) || throw(ArgumentError("ichunk must be less or equal to the length of the ChunksIterator"))
if C == FixedCount
n = c.n
size = nothing
n >= 1 || throw(ArgumentError("n must be >= 1"))
elseif C == FixedSize
n = nothing
size = c.size
size >= 1 || throw(ArgumentError("size must be >= 1"))
l = length(c.collection)
size = min(l, size) # handle size>length(c.collection)
n = cld(l, size)
if length(c) > 0
if C == FixedCount
n = c.n
size = nothing
elseif C == FixedSize
n = nothing
size = c.size
l = length(c.collection)
size = min(l, size) # handle size>length(c.collection)
n = cld(l, size)
end
else
n = 0
end
ichunk <= n || throw(ArgumentError("index must be less or equal to number of chunks ($n)"))
1 <= ichunk <= n || throw(BoundsError(c, ichunk))
return _getchunkindices(C, S, c.collection, ichunk; n, size)
end

Expand Down
2 changes: 2 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ end
@test_throws ArgumentError f(1:10; n=5, size=2)
@test_throws ArgumentError f(1:10; n=5, size=20)
@test_throws TypeError f(1:10; n=2, split=:scatter) # not supported anymore
@test_throws BoundsError f(0:-1; n=2)[1]
@test_throws BoundsError f(1:10; n=2)[3]
end
end
end
Expand Down
Loading