Skip to content

Embrace pairs for show and construction #179

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.

Sign up for GitHub

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

Closed
timholy wants to merge 2 commits into from
Closed

Embrace pairs for show and construction #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fbfa7f6
Embrace pairs for `show` and construction
timholy Jan 3, 2021
b6f7d83
Fix busted doctest on Julia 1
timholy Jan 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions docs/src/internals.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type:

```jldoctest oa
julia> ax = axes(oa, 2)
OffsetArrays.IdOffsetRange(5:6)
(5 => 5):(6 => 6)
```

This has a similar design to `Base.IdentityUnitRange` that `ax[x] == x` always holds.
Expand All @@ -61,10 +61,10 @@ This property makes sure that they tend to be their own axes:

```jldoctest oa
julia> axes(ax)
(OffsetArrays.IdOffsetRange(5:6),)
((5 => 5):(6 => 6),)

julia> axes(ax[ax])
(OffsetArrays.IdOffsetRange(5:6),)
((5 => 5):(6 => 6),)
```

This example of indexing is [idempotent](https://en.wikipedia.org/wiki/Idempotence).
Expand All @@ -80,7 +80,7 @@ julia> oa2 = OffsetArray([5, 10, 15, 20], 0:3)
20

julia> ax2 = axes(oa2, 1)
OffsetArrays.IdOffsetRange(0:3)
(0 => 0):(3 => 3)

julia> oa2[2]
15
Expand Down Expand Up @@ -112,22 +112,23 @@ cases that you should be aware of, especially when you are working with multi-di
One such cases is `getindex`:

```jldoctest getindex; setup = :(using OffsetArrays)
julia> Ao = zeros(-3:3, -3:3); Ao[:] .= 1:49;
julia> Ao = zeros(-3:3, -3:3); Ao[:] .= 1:49; axes(Ao)
((-3 => -3):(3 => 3), (-3 => -3):(3 => 3))

julia> Ao[-3:0, :] |> axes # the first dimension does not preserve offsets
(OffsetArrays.IdOffsetRange(1:4), OffsetArrays.IdOffsetRange(-3:3))
((1 => 1):(4 => 4), (-3 => -3):(3 => 3))

julia> Ao[-3:0, -3:3] |> axes # neither dimensions preserve offsets
julia> Ao[-3:0, -3:3] |> axes # neither dimension preserves offsets
(Base.OneTo(4), Base.OneTo(7))

julia> Ao[axes(Ao)...] |> axes # offsets are preserved
(OffsetArrays.IdOffsetRange(-3:3), OffsetArrays.IdOffsetRange(-3:3))
((-3 => -3):(3 => 3), (-3 => -3):(3 => 3))

julia> Ao[:] |> axes # This is linear indexing
(Base.OneTo(49),)
```

Note that if you pass a `UnitRange`, the offsets in corresponding dimension will not be preserved.
Note that if you pass a `UnitRange`, the offsets in the corresponding dimension will not be preserved.
This might look weird at first, but since it follows the `a[ax][i] == a[ax[i]]` rule, it is not a
bug.

Expand All @@ -138,7 +139,7 @@ julia> Ao[I, 0][1] == Ao[I[1], 0]
true

julia> ax = axes(Ao, 1) # ax starts at index -3
OffsetArrays.IdOffsetRange(-3:3)
(-3 => -3):(3 => 3)

julia> Ao[ax, 0][1] == Ao[ax[1], 0]
true
Expand All @@ -164,15 +165,15 @@ julia> a = zeros(3, 3);
julia> oa = OffsetArray(a, ZeroBasedIndexing());

julia> axes(oa)
(OffsetArrays.IdOffsetRange(0:2), OffsetArrays.IdOffsetRange(0:2))
((0 => 0):(2 => 2), (0 => 0):(2 => 2))
```

In this example we had to define the action of `to_indices` as the type `ZeroBasedIndexing` did not have a familiar hierarchy. Things are even simpler if we subtype `AbstractUnitRange`, in which case we need to define `first` and `length` for the custom range to be able to use it as an axis:

```jldoctest; setup = :(using OffsetArrays)
julia> struct ZeroTo <: AbstractUnitRange{Int}
n :: Int
ZeroTo(n) = new(n < 0 ? -1 : n)
n :: Int
ZeroTo(n) = new(n < 0 ? -1 : n)
end

julia> Base.first(::ZeroTo) = 0
Expand All @@ -182,7 +183,7 @@ julia> Base.length(r::ZeroTo) = r.n + 1
julia> oa = OffsetArray(zeros(2,2), ZeroTo(1), ZeroTo(1));

julia> axes(oa)
(OffsetArrays.IdOffsetRange(0:1), OffsetArrays.IdOffsetRange(0:1))
((0 => 0):(1 => 1), (0 => 0):(1 => 1))
```

Note that zero-based indexing may also be achieved using the pre-defined type [`OffsetArrays.Origin`](@ref).
Note that zero-based indexing may also be achieved using the pre-defined type [`OffsetArrays.Origin`](@ref).
51 changes: 42 additions & 9 deletions src/axes.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,55 @@ i.e., it's the "identity," which is the origin of the "Id" in `IdOffsetRange`.
# Examples

The most common case is shifting a range that starts at 1 (either `1:n` or `Base.OneTo(n)`):
```jldoctest; setup=:(import OffsetArrays)
```jldoctest idorange; setup=:(import OffsetArrays)
julia> ro = OffsetArrays.IdOffsetRange(1:3, -2)
OffsetArrays.IdOffsetRange(-1:1)
(-1 => -1):(1 => 1)
```

julia> axes(ro, 1)
OffsetArrays.IdOffsetRange(-1:1)
You can think of this display as indicating that an index of -1 maps to -1, and an index of 1 maps to 1.

```jldoctest idorange
julia> ro[-1]
-1

julia> ro[3]
ERROR: BoundsError: attempt to access 3-element UnitRange{$Int} at index [5]

julia> axes(ro, 1) # `axes` is Idempotent
(-1 => -1):(1 => 1)
```

If the range doesn't start at 1, the values may be different from the indices:
```jldoctest; setup=:(import OffsetArrays)
julia> ro = OffsetArrays.IdOffsetRange(11:13, -2)
OffsetArrays.IdOffsetRange(9:11)

julia> axes(ro, 1) # 11:13 is indexed by 1:3, and the offset is also applied to the axes
OffsetArrays.IdOffsetRange(-1:1)
(-1 => 9):(1 => 11)

julia> ro[-1]
9

julia> ro[3]
ERROR: BoundsError: attempt to access 3-element UnitRange{$Int} at index [5]

julia> axes(ro, 1) # 11:13 is indexed by 1:3, and the offset is also applied to the axes
(-1 => -1):(1 => 1)
```

You can construct these ranges as they are displayed:

```jldoctest; setup=(import OffsetArrays), filter=r", ?U"
julia> r = (0=>8):(3=>11)
(0 => 8):(3 => 11)

julia> typeof(r)
OffsetArrays.IdOffsetRange{$Int, UnitRange{$Int}}

julia> for p in pairs(r)
println(p)
end
0 => 8
1 => 9
2 => 10
3 => 11
```

# Extended help
Expand Down Expand Up @@ -104,6 +126,14 @@ end
IdOffsetRange(r::IdOffsetRange) = r
IdOffsetRange(r::IdOffsetRange, offset::Integer) = typeof(r)(r.parent, offset + r.offset)

function Base.:(:)((istart,rstart)::Pair{Int,Int}, (istop,rstop)::Pair{Int,Int})
throw_argerr(istart, istop, rstart, rstop) = throw(ArgumentError("indices and values must have the same length, got $istart:$istop (length $(istop-istart+1)) and $rstart:$rstop (length $(rstop-rstart+1)), respectively"))

istop - istart == rstop - rstart || throw_argerr(istart, istop, rstart, rstop)
offset = istart - 1
return IdOffsetRange(rstart-offset : rstop-offset, offset)
end
Copy link
Member

@johnnychen94 johnnychen94 Jan 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If we're interpreting => as key => value pair, then I think 2:4 => 5:7 should be equivalent to (2 => 5):(4 => 7) instead of (2 => 4):(5 => 7)

Hence if we introduce this in Base, it should be something like

Base.:(:)(src::Pair, dest::Pair) = src.first:dest.first => src.second:dest.second

Forcing it to return an IdOffsetRange makes little sense to me as it breaks the 2:4 => 5:7 and (2 => 5):(4 => 7) equivalence by adding a new range meaning (IdOffsetRange) to it. I mean, it's as little sense as making (1, 1):(4, 4) to be CartesianIndex(2, 2):CartesianIndex(4, 4).

It might be better to adjust this PR to:

julia> IdOffsetRange(4:6, 1)
IdOffsetRange((2 => 5):(4 => 7))

Copy link
Member Author

@timholy timholy Jan 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This PR doesn't "do anything" with 2:4 => 5:7. A Pair-of-ranges seems like a well-defined concept. Conversely, a range-of-pairs currently is an error:

julia> 2:4 => 5:7    # no error
2:4 => 5:7

julia> (2=>5) : (4=>7)
ERROR: MethodError: no method matching -(::Pair{Int64, Int64}, ::Pair{Int64, Int64})
Stacktrace:
 [1] (::Colon)(start::Pair{Int64, Int64}, stop::Pair{Int64, Int64})
   @ Base ./range.jl:7
 [2] top-level scope
   @ REPL[1]:1

Consequently we have the opportunity to make it mean something. We could even rename IdOffsetRange to RangePair. They're "Id" (idempotent) only when the parent range starts at 1:

julia> r = OffsetArrays.IdOffsetRange(1:5, -3)
(-2 => -2):(2 => 2)

julia> axes(r)
((-2 => -2):(2 => 2),)

julia> r = OffsetArrays.IdOffsetRange(2:6, -3)
(-2 => -1):(2 => 3)

julia> axes(r)
((-2 => -2):(2 => 2),)


# TODO: uncomment these when Julia is ready
# # Conversion preserves both the values and the indexes, throwing an InexactError if this
# # is not possible.
Expand Down Expand Up @@ -167,7 +197,10 @@ Broadcast.broadcasted(::Base.Broadcast.DefaultArrayStyle{1}, ::typeof(+), r::IdO
Broadcast.broadcasted(::Base.Broadcast.DefaultArrayStyle{1}, ::typeof(+), x::Integer, r::IdOffsetRange{T}) where T =
IdOffsetRange{T}(x .+ r.parent, r.offset)

Base.show(io::IO, r::IdOffsetRange) = print(io, "OffsetArrays.IdOffsetRange(",first(r), ':', last(r),")")
function Base.show(io::IO, r::IdOffsetRange)
axr = axes(r, 1)
print(io, "(",first(axr)=>first(r), "):(", last(axr)=>last(r),")")
end

# Optimizations
@inline Base.checkindex(::Type{Bool}, inds::IdOffsetRange, i::Real) = Base.checkindex(Bool, inds.parent, i - inds.offset)
Expand Down
36 changes: 22 additions & 14 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,14 @@ end
r = IdOffsetRange(IdOffsetRange(3:5, 2), 1)
@test parent(r) isa UnitRange

# Pair construction
rp = (2=>3):(5=>6)
@test first(rp) === 3
@test last(rp) === 6
@test firstindex(rp) === 2
@test lastindex(rp) === 5
@test_throws ArgumentError("indices and values must have the same length, got 0:1 (length 2) and 5:7 (length 3), respectively") (0=>5):(1=>7)

# conversion preserves both the values and the axes, throwing an error if this is not possible
@test @inferred(oftype(ro, ro)) === ro
@test @inferred(convert(OffsetArrays.IdOffsetRange{Int}, ro)) === ro
Expand Down Expand Up @@ -199,19 +207,19 @@ end
@testset "OffsetVector" begin
# initialization
one_based_axes = [
(Base.OneTo(4), ),
(1:4, ),
(CartesianIndex(1):CartesianIndex(4), ),
(IdentityUnitRange(1:4), ),
(IdOffsetRange(1:4),),
(Base.OneTo(4), ),
(1:4, ),
(CartesianIndex(1):CartesianIndex(4), ),
(IdentityUnitRange(1:4), ),
(IdOffsetRange(1:4),),
(IdOffsetRange(3:6, -2),)
]

offset_axes = [
(-1:2, ),
(CartesianIndex(-1):CartesianIndex(2), ),
(IdentityUnitRange(-1:2), ),
(IdOffsetRange(-1:2),),
(-1:2, ),
(CartesianIndex(-1):CartesianIndex(2), ),
(IdentityUnitRange(-1:2), ),
(IdOffsetRange(-1:2),),
(IdOffsetRange(3:6, -4),)
]

Expand Down Expand Up @@ -324,7 +332,7 @@ end

@testset "OffsetMatrix" begin
# initialization

one_based_axes = [
(Base.OneTo(4), Base.OneTo(3)),
(1:4, 1:3),
Expand Down Expand Up @@ -573,7 +581,7 @@ end
end
@testset "TupleOfRanges" begin
Base.to_indices(A, inds, t::Tuple{TupleOfRanges{N}}) where {N} = t
OffsetArrays.AxisConversionStyle(::Type{TupleOfRanges{N}}) where {N} =
OffsetArrays.AxisConversionStyle(::Type{TupleOfRanges{N}}) where {N} =
OffsetArrays.TupleOfRanges()

Base.convert(::Type{Tuple{Vararg{AbstractUnitRange{Int}}}}, t::TupleOfRanges) = t.x
Expand All @@ -584,7 +592,7 @@ end
@test axes(oa) == inds.x
end
@testset "NewColon" begin
Base.to_indices(A, inds, t::Tuple{NewColon,Vararg{Any}}) =
Base.to_indices(A, inds, t::Tuple{NewColon,Vararg{Any}}) =
(_uncolon(inds, t), to_indices(A, Base.tail(inds), Base.tail(t))...)

_uncolon(inds::Tuple{}, I::Tuple{NewColon, Vararg{Any}}) = OneTo(1)
Expand Down Expand Up @@ -916,9 +924,9 @@ end
a = OffsetArray([1 2; 3 4], -1:0, 5:6)
io = IOBuffer()
show(io, axes(a, 1))
@test String(take!(io)) == "OffsetArrays.IdOffsetRange(-1:0)"
@test String(take!(io)) == "(-1 => -1):(0 => 0)"
show(io, axes(a, 2))
@test String(take!(io)) == "OffsetArrays.IdOffsetRange(5:6)"
@test String(take!(io)) == "(5 => 5):(6 => 6)"

@test Base.inds2string(axes(a)) == Base.inds2string(map(UnitRange, axes(a)))

Expand Down