Define Replicate(), Symmetric() and other border options like Inner() instead of using strings

[juliohm]

Can you please confirm the new syntax for the following two operations:

img = ones(10,10,10)
kern = ones(3,3,3)

imfilter_fft(img, kern, "inner")
imfilter_fft(img, kern, "symmetric")

Am I correct to say that the new version is:

imfilter(img, centered(kern), Inner(), Algorithm.FFT())
imfilter(img, centered(kern), "symmetric", Algorithm.FFT())

Why Inner() is treated with a separate type unlike other options that are triggered with strings?

[juliohm]

I have just submitted a journal paper regarding my ImageQuilting.jl package. I need to fix this issue quickly to avoid problems with people trying to use the code for the first time.

[timholy]

Yes, those are correct.

Why Inner() is treated with a separate type unlike other options that are triggered with strings?

Because with Inner you'll return an OffsetArray whereas with the others you'll return an Array. The algorithm would not be type-stable if we passed "inner" as a string.

[juliohm]

Hi Tim,

You probably have thought about this interface very carefully, but from the user side this is very tricky. Returning an OffsetArray puts responsibility on the user that now has to check if he has an Array or OffsetArray. For instance, I cannot assume anymore I will always get an Array, because sometimes I use symmetric and sometimes I use Inner(). It is counter-intuitive to have the same conceptual filtering operation returning different types.

Now, related to this issue specifically, can't we make every border scheme a type to be consistent? Symmetric(), Replicate(), would that solve the type-instability too?

[timholy]

Cross-reference JuliaArrays/OffsetArrays.jl#18 (comment). See docs on the "generic array interface" at http://docs.julialang.org/en/latest/devdocs/offset-arrays.html#Arrays-with-custom-indices-1.

[juliohm]

Thank you Tim, what about replacing the strings "symmetric", "replicate" by types like Inner()? Would that solve type instability too? It would definitely make the interface consistent.

[timholy]

There's already Pad(:replicate). At one point I made the padding style Pad{:replicate}(), but then I realized that they all produce the same output type anyway. So I decided to preserve the strings interface.

We could make Replicate() return "replicate". Would that be viewed as being nicer?

[juliohm]

Hi Tim, ideally these would be their own types in case someone wants to specialize some super optimal implementation in the future, but having Replicate() return the string "replicate" is already more consistent from the user's perspective, definitely.

[juliohm]

I am finally putting together all the pieces of this great work. Thanks Tim for your patience.

Everything is consistent as we can simply do imfilter(img, centered(kern), Pad(:symmetric)) and use the Pad type instead of the string.