update to julia 1.1

* maybe_copy doesn't explain its usage * replace Slice by OffsetArrays.IdentityUnitRange JuliaArrays/OffsetArrays.jl#62
Evizero · Dec 16, 2019 · dc3682e · dc3682e
1 parent 23ddbd1
commit dc3682e
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 45 deletions.
diff --git a/src/Augmentor.jl b/src/Augmentor.jl
@@ -9,13 +9,18 @@ using CoordinateTransformations
 using Rotations
 using Interpolations
 using StaticArrays
-using OffsetArrays
 using IdentityRanges
 using MLDataPattern
 using ComputationalResources
 using FileIO
 using Base.PermutedDimsArrays: PermutedDimsArray
 using LinearAlgebra
+using OffsetArrays
+
+# axes(::OffsetArray) changes from Base.Slice to Base.IdentityUnitRange in julia 1.1
+# https://github.com/JuliaArrays/OffsetArrays.jl/pull/62
+# TODO: switch to Base.IdentityUnitRange when we decide to drop 1.0 compatibility
+using OffsetArrays: IdentityUnitRange
 
 export
 

diff --git a/src/operation.jl b/src/operation.jl
@@ -63,7 +63,7 @@ for FUN in (:applyeager, :applylazy, :applypermute,
 end
 
 function applyeager(op::Operation, img::AbstractArray, param)
-    maybe_copy(applylazy(op, img, param))
+    contiguous(applylazy(op, img, param))
 end
 
 function applyaffineview(op::Operation, img::AbstractArray, param)

diff --git a/src/operations/cache.jl b/src/operations/cache.jl
@@ -53,7 +53,7 @@ pl = ElasticDistortion(3,3) |> zeros(20,20) |> Rotate(-10:10)
 """
 struct CacheImage <: ImageOperation end
 
-applyeager(op::CacheImage, img::AbstractArray, param) = maybe_copy(img)
+applyeager(op::CacheImage, img::AbstractArray, param) = contiguous(img)
 
 function showconstruction(io::IO, op::CacheImage)
     print(io, typeof(op).name.name, "()")

diff --git a/src/operations/convert.jl b/src/operations/convert.jl
@@ -52,7 +52,7 @@ end
 @inline supports_lazy(::Type{<:ConvertEltype}) = true
 
 function applyeager(op::ConvertEltype{T}, img::AbstractArray, param) where T
-    maybe_copy(convert(AbstractArray{T}, img))
+    contiguous(convert(AbstractArray{T}, img))
 end
 
 function applylazy(op::ConvertEltype{T}, img::AbstractArray, param) where T

diff --git a/src/operations/mapfun.jl b/src/operations/mapfun.jl
@@ -51,7 +51,7 @@ end
 @inline supports_lazy(::Type{<:MapFun}) = true
 
 function applyeager(op::MapFun, img::AbstractArray, param)
-    maybe_copy(map(op.fun, img))
+    contiguous(map(op.fun, img))
 end
 
 function applylazy(op::MapFun, img::AbstractArray, param)
@@ -130,7 +130,7 @@ end
 
 function applyeager(op::AggregateThenMapFun, img::AbstractArray, param)
     agg = op.aggfun(img)
-    maybe_copy(map(x -> op.mapfun(x, agg), img))
+    contiguous(map(x -> op.mapfun(x, agg), img))
 end
 
 function applylazy(op::AggregateThenMapFun, img::AbstractArray, param)

diff --git a/src/operations/noop.jl b/src/operations/noop.jl
@@ -15,7 +15,7 @@ struct NoOp <: AffineOperation end
 
 # TODO: implement method for n-dim arrays
 toaffinemap(::NoOp, img::AbstractMatrix) = AffineMap(@SMatrix([1. 0; 0 1.]), @SVector([0.,0.]))
-applyeager(::NoOp, img::AbstractArray, param) = maybe_copy(img)
+applyeager(::NoOp, img::AbstractArray, param) = contiguous(img)
 applylazy(::NoOp, img::AbstractArray, param) = img
 
 function applyview(::NoOp, img::AbstractArray, param)

diff --git a/src/utils.jl b/src/utils.jl
@@ -32,13 +32,21 @@ function safe_rand(args...)
 end
 
 # --------------------------------------------------------------------
+"""
+    contiguous(A::AbstractArray)
+    contiguous(A::Tuple)
+
+Return a memory contiguous array for better performance.
 
-@inline maybe_copy(A::OffsetArray) = A
-@inline maybe_copy(A::Array) = A
-@inline maybe_copy(A::SArray) = A
-@inline maybe_copy(A::MArray) = A
-@inline maybe_copy(A::AbstractArray) = match_idx(collect(A), axes(A))
-@inline maybe_copy(A::Tuple) = map(maybe_copy, A)
+Data copy only happens when necessary. For example, views returned by `view`,
+`permuteddimsview` are such cases.
+"""
+@inline contiguous(A::OffsetArray) = A
+@inline contiguous(A::Array) = A
+@inline contiguous(A::SArray) = A
+@inline contiguous(A::MArray) = A
+@inline contiguous(A::AbstractArray) = match_idx(collect(A), axes(A))
+@inline contiguous(A::Tuple) = map(contiguous, A)
 
 # --------------------------------------------------------------------
 
@@ -48,10 +56,15 @@ end
 @inline _plain_array(A::MArray) = A
 @inline _plain_array(A::Tuple) = map(_plain_array, A)
 # avoid recursion
-@inline plain_array(A) = _plain_array(maybe_copy(A))
+@inline plain_array(A) = _plain_array(contiguous(A))
 
 # --------------------------------------------------------------------
 
+"""
+    plain_axes(A::AbstractArray)
+
+Generate a 1-based array from `A` without data copy.
+"""
 @inline plain_axes(A::Array) = A
 @inline plain_axes(A::OffsetArray) = parent(A)
 @inline plain_axes(A::AbstractArray) = _plain_axes(A, axes(A))
@@ -65,7 +78,7 @@ end
     view(A, axes(A)...)
 end
 
-@inline function _plain_axes(A::AbstractArray{T,N}, ids::NTuple{N,Base.Slice}) where {T, N}
+@inline function _plain_axes(A::AbstractArray{T,N}, ids::NTuple{N, IdentityUnitRange}) where {T, N}
     view(A, map(i->i.indices, ids)...)
 end
 
@@ -76,7 +89,7 @@ end
 # --------------------------------------------------------------------
 
 @inline match_idx(buffer::AbstractArray, inds::Tuple) = buffer
-@inline match_idx(buffer::Union{Array,SubArray}, inds::NTuple{N,Union{UnitRange,Base.Slice{<:UnitRange}}}) where {N} =
+@inline match_idx(buffer::Union{Array,SubArray}, inds::NTuple{N,Union{UnitRange, IdentityUnitRange}}) where {N} =
     OffsetArray(buffer, inds)
 
 # --------------------------------------------------------------------

diff --git a/test/operations/tst_convert.jl b/test/operations/tst_convert.jl
@@ -57,7 +57,7 @@
         let img = @inferred(Augmentor.applylazy(ConvertEltype(Gray{Float32}), OffsetArray(rect,-2,-1)))
             @test parent(parent(img)) === rect
             @test typeof(img) <: ReadonlyMappedArray{Gray{Float32},2}
-            @test axes(img) === (Base.Slice(-1:0), Base.Slice(0:2))
+            @test axes(img) === (OffsetArrays.IdentityUnitRange(-1:0), OffsetArrays.IdentityUnitRange(0:2))
             @test img[0,0] isa Gray{Float32}
             @test collect(img) == convert.(Gray{Float32}, rect)
         end

diff --git a/test/operations/tst_mapfun.jl b/test/operations/tst_mapfun.jl
@@ -54,7 +54,7 @@
         res = @inferred(Augmentor.applylazy(MapFun(x->x-RGB(.1,.1,.1)), rgb_rect))
         @test res == mappedarray(x->x-RGB(.1,.1,.1), rgb_rect)
         res = @inferred(Augmentor.applylazy(MapFun(x->x-RGB(.1,.1,.1)), OffsetArray(rgb_rect,-2,-1)))
-        @test axes(res) === Base.Slice.((-1:0, 0:2))
+        @test axes(res) === OffsetArrays.IdentityUnitRange.((-1:0, 0:2))
         @test @inferred(getindex(res,0,0)) isa RGB{Float64}
         @test res == mappedarray(x->x-RGB(.1,.1,.1), OffsetArray(rgb_rect,-2,-1))
         @test typeof(res) <: MappedArrays.ReadonlyMappedArray{ColorTypes.RGB{Float64}}

diff --git a/test/tst_utils.jl b/test/tst_utils.jl
@@ -23,53 +23,53 @@ end
     @test typeof(num) <: Vector{Float64}
 end
 
-@testset "maybe_copy" begin
+@testset "contiguous" begin
     A = [1 2 3; 4 5 6; 7 8 9]
     Ao = OffsetArray(A, (-2,-1))
-    @test @inferred(Augmentor.maybe_copy(A)) === A
-    @test @inferred(Augmentor.maybe_copy(Ao)) === Ao
+    @test @inferred(Augmentor.contiguous(A)) === A
+    @test @inferred(Augmentor.contiguous(Ao)) === Ao
     let Ast = @SMatrix [1 2 3; 4 5 6; 7 8 9]
-        @test @inferred(Augmentor.maybe_copy(Ast)) === Ast
+        @test @inferred(Augmentor.contiguous(Ast)) === Ast
     end
     let v = view(A, 2:3, 1:2)
-        @test @inferred(Augmentor.maybe_copy(v)) == A[2:3, 1:2]
-        @test typeof(Augmentor.maybe_copy(v)) <: Array
+        @test @inferred(Augmentor.contiguous(v)) == A[2:3, 1:2]
+        @test typeof(Augmentor.contiguous(v)) <: Array
     end
     let v = view(OffsetArray(A, (-2,-1)), 0:1, 0:1)
-        @test @inferred(Augmentor.maybe_copy(v)) == A[2:3, 1:2]
-        @test typeof(Augmentor.maybe_copy(v)) <: Array
+        @test @inferred(Augmentor.contiguous(v)) == A[2:3, 1:2]
+        @test typeof(Augmentor.contiguous(v)) <: Array
     end
     let v = view(A, IdentityRange(2:3), IdentityRange(1:2))
-        @test @inferred(Augmentor.maybe_copy(v)) == OffsetArray(A[2:3,1:2], (1, 0))
-        @test typeof(Augmentor.maybe_copy(v)) <: OffsetArray
+        @test @inferred(Augmentor.contiguous(v)) == OffsetArray(A[2:3,1:2], (1, 0))
+        @test typeof(Augmentor.contiguous(v)) <: OffsetArray
     end
     let v = channelview(rect)
-        @test @inferred(Augmentor.maybe_copy(v)) == channelview(rect)
-        @test typeof(Augmentor.maybe_copy(v)) <: Array
+        @test @inferred(Augmentor.contiguous(v)) == channelview(rect)
+        @test typeof(Augmentor.contiguous(v)) <: Array
     end
     let p = permuteddimsview(A, (2,1))
-        @test @inferred(Augmentor.maybe_copy(p)) == A'
-        @test typeof(Augmentor.maybe_copy(p)) <: Array
+        @test @inferred(Augmentor.contiguous(p)) == A'
+        @test typeof(Augmentor.contiguous(p)) <: Array
     end
     let p = permuteddimsview(Ao, (2,1))
-        @test @inferred(Augmentor.maybe_copy(p)) == Ao'
-        @test typeof(Augmentor.maybe_copy(p)) <: OffsetArray
+        @test @inferred(Augmentor.contiguous(p)) == Ao'
+        @test typeof(Augmentor.contiguous(p)) <: OffsetArray
     end
     let p = view(permuteddimsview(A, (2,1)), IdentityRange(2:3), IdentityRange(1:2))
-        @test @inferred(Augmentor.maybe_copy(p)) == OffsetArray(A'[2:3, 1:2],1,0)
-        @test typeof(Augmentor.maybe_copy(p)) <: OffsetArray
+        @test @inferred(Augmentor.contiguous(p)) == OffsetArray(A'[2:3, 1:2],1,0)
+        @test typeof(Augmentor.contiguous(p)) <: OffsetArray
     end
     let Aa = Augmentor.prepareaffine(A)
-        @test @inferred(Augmentor.maybe_copy(Aa)) == OffsetArray(A, (0,0))
-        @test typeof(Augmentor.maybe_copy(Aa)) <: OffsetArray
+        @test @inferred(Augmentor.contiguous(Aa)) == OffsetArray(A, (0,0))
+        @test typeof(Augmentor.contiguous(Aa)) <: OffsetArray
     end
     let Ar = reshape(view(A,:,:),1,3,3)
-        @test @inferred(Augmentor.maybe_copy(Ar)) == reshape(A,1,3,3)
-        @test typeof(Augmentor.maybe_copy(Ar)) <: Array{Int,3}
+        @test @inferred(Augmentor.contiguous(Ar)) == reshape(A,1,3,3)
+        @test typeof(Augmentor.contiguous(Ar)) <: Array{Int,3}
     end
     let Ar = reshape(view(Ao,:,:),1,3,3)
-        @test @inferred(Augmentor.maybe_copy(Ar)) == reshape(A,1,3,3)
-        @test typeof(Augmentor.maybe_copy(Ar)) <: Array{Int,3}
+        @test @inferred(Augmentor.contiguous(Ar)) == reshape(A,1,3,3)
+        @test typeof(Augmentor.contiguous(Ar)) <: Array{Int,3}
     end
 end
 
@@ -157,19 +157,19 @@ end
     A = [1 2 3; 4 5 6; 7 8 9]
     @test @inferred(Augmentor.match_idx(A, axes(A))) === A
     let img = @inferred Augmentor.match_idx(A, (2:4, 2:4))
-        @test axes(img) === Base.Slice.((2:4, 2:4))
+        @test axes(img) === OffsetArrays.IdentityUnitRange.((2:4, 2:4))
         @test typeof(img) <: OffsetArray
     end
     let B = view(A,1:3,1:3)
         @test @inferred(Augmentor.match_idx(B, axes(B))) === B
     end
     let B = view(A,1:3,1:3)
         img = @inferred(Augmentor.match_idx(B, B.indices))
-        @test axes(img) === Base.Slice.((1:3, 1:3))
+        @test axes(img) === OffsetArrays.IdentityUnitRange.((1:3, 1:3))
         @test typeof(img) <: OffsetArray
     end
     let img = @inferred Augmentor.match_idx(view(A,1:3,1:3), (2:4,2:4))
-        @test axes(img) === Base.Slice.((2:4, 2:4))
+        @test axes(img) === OffsetArrays.IdentityUnitRange.((2:4, 2:4))
         @test typeof(img) <: OffsetArray
     end
     let C = Augmentor.prepareaffine(A)