Fix iteration for 0-dimensional disk arrays#293
Fix iteration for 0-dimensional disk arrays#293sethaxen wants to merge 4 commits intoJuliaIO:mainfrom
Conversation
src/iterator.jl
Outdated
| end | ||
| end | ||
| end | ||
| _iterate_disk(a::AbstractArray{<:Any,0}, i::Int=1) = i == 1 ? (a[], 2) : nothing |
There was a problem hiding this comment.
Why is the index restricted to an Int?
According to the docstring of iterate this is supposed to be an Integer and that also works for normal Arrays.
There was a problem hiding this comment.
Ah, I see now it doesn't even require the state be an Int (that's just for AbstractString). iterate(::Array, i)'s implementation has no type constraint on i and just uses it for bounds checking and indexing. Meanwhile, valid index types for AbstractArrays are CartesianIndex (implicitly not supported here due to the bounds check) or "non-boolean integer" (https://docs.julialang.org/en/v1/manual/arrays/#man-supported-index-types), so I think this would be more consistent with the Base method, as it will error whenever the Base method would error (namely, when a CartesianIndex or Bool is provided):
| _iterate_disk(a::AbstractArray{<:Any,0}, i::Int=1) = i == 1 ? (a[], 2) : nothing | |
| _iterate_disk(a::AbstractArray{<:Any,0}, i=1) = i == 1 ? (@inbounds a[i], 2) : nothing |
There was a problem hiding this comment.
There is one equal sign to much in your function definition. Apart from that this looks good.
There was a problem hiding this comment.
Ah, seems removing the Int specialization causes ambiguity with the other method. I could also remove that specialization or add it back to this method.
Fixes #289