JuliaImages / ImageFiltering.jl

Julia implementations of multidimensional array convolution and nonlinear stencil operations
Other
99 stars 49 forks source link

correct the behavior of centered for OffsetArray #104

Closed johnnychen94 closed 5 years ago

johnnychen94 commented 5 years ago

This patch generalizes centered(A::AbstractArray) by not assuming indexes of A starting at 1.

A side effect of this patch is to corrects the behavior of imfilter with accident usage of centered(::OffsetArray)

Before:

julia> x = testimage("cameraman")

julia> k = KernelFactors.gaussian(1.5, 11)
OffsetArray(::Array{Float64,1}, -5:5) with eltype Float64 with indices -5:5:

julia> a = imfilter(x, kernelfactors((k, k)))

julia> b = imfilter(x, kernelfactors((centered(k), centered(k))))

julia> a - b

image

After:

julia> gray.(sum(abs.(a-b)))
0.0
codecov[bot] commented 5 years ago

Codecov Report

Merging #104 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
+ Coverage   81.58%   81.59%   +0.01%     
==========================================
  Files           9        9              
  Lines        1200     1201       +1     
==========================================
+ Hits          979      980       +1     
  Misses        221      221
Impacted Files Coverage Δ
src/utils.jl 73.68% <100%> (+0.46%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0ae7df1...4d37ff8. Read the comment docs.

codecov[bot] commented 5 years ago

Codecov Report

Merging #104 into master will increase coverage by 0.86%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
+ Coverage   81.58%   82.44%   +0.86%     
==========================================
  Files           9        9              
  Lines        1200     1202       +2     
==========================================
+ Hits          979      991      +12     
+ Misses        221      211      -10
Impacted Files Coverage Δ
src/utils.jl 74.13% <100%> (+0.92%) :arrow_up:
src/kernelfactors.jl 80.15% <0%> (+7.93%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0ae7df1...5c0cab3. Read the comment docs.

zygmuntszpak commented 5 years ago

In summary, this pull-request ensures that if you call centered on an OffsetArray it will adjust the axes of the original data array that was being wrapped.

For example,

axes(centered(collect(1:5))) # (Base.IdentityUnitRange(-2:2),)

axes(OffsetArray(collect(1:5),-1)) # (Base.IdentityUnitRange(0:4),)

axes(centered(OffsetArray(collect(1:5),-1)))  # (Base.IdentityUnitRange(-3:1),)  (Without this pull-request)

axes(centered(OffsetArray(collect(1:5),-1)))  # (Base.IdentityUnitRange(-2:2),)  (Thanks to this pull-request)

@timholy Are you OK with this new behaviour?

timholy commented 5 years ago

Thanks for doing this!