upgrade legacy methods with cleaner keyword alternative

[johnnychen94]

Julia's keyword argument is not very performant before Julia 1.0. IIUC, this is why this package used a lot of multiple dispatch tricks on positional arguments to work around the performance overhead. However, the unfortunate part of the non-orthogonal positional argument is that it easily blows the number of methods when more possibility comes and makes it prone to ambiguities(#115) and StackOverflow errors(#57), not to mention the maintenance burden (really hard to figure out the calling order).

This PR rewrites the internal interfaces to use keyword parameter method=Linear() and fillvalue = _default_fillvalue(T).

API changes

All degree and fill positional arguments are deprecated in favor of their keyword version method and fillvalue. For simplicity, I didn't add type annotations here.

• imrotate(img, sizes, [degree], [fill]) -> imrotate(img, sizes; [method], [fillvalue])
• warp(img, tform, indices, [degree], [fill]) -> warp(img, tform, indices; [method], [fillvalue])
• (Behavior changed) WarpedView(img, tform, indices) -> WarpedView(img, tform, indices; [method], [fillvalue])
  - [ ] (Behavior changed) InvWarpedView(img, tform, indices) -> InvWarpedView(img, tform, indices; [method], [fillvalue])
• deprecate warpedview in favor of WarpedView
• deprecate invwarpedview in favor of InvWarpedView

Old usages will still be tested in test/deprecated.jl so that it hopefully doesn't break people's code.

Internal function changes:

• box_extrapolation(img, [degree], [fill]) -> `box_extrapolation(img; method, fillvalue)
• rename: _default_fill -> _default_fillvalue
• deprecate IdentityRanges.IdentityRange in favor of Base.IdentityUnitRange (deprecate this package in favor of Base.IdentityUnitRange JuliaArrays/IdentityRanges.jl#14)

No deprecation warnings will be provided for them, and there will be no depwarn tests in test/deprecated.jl.

code reorganization

This part is not yet done.

• All backward compatibility codes go to compat.jl
• All deprecations codes go to deprecated.jl

Documentation

• update documentation
• add docs CI (future PR)

The docstrings will be rewritten in a more structured style outlined in JuliaImages/Images.jl#790. In the next PR, I'll set up the documentation CI.

Test changes

• update summary test (future PR)

summary test is prone to internal changes and can be very different across multiple Julia versions. Thus they'll be replaced with other checks.

---

After this PR, I'll set up the documentation and then tag v0.9.0. I still have some other schedules, but hopefully, I'll make it before May.

cc: @andrew-saydjari @Evizero

[codecov]

Codecov Report

Merging #116 (3fe10ed) into master (9257562) will decrease coverage by 0.92%.
The diff coverage is 97.14%.

@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
- Coverage   95.02%   94.09%   -0.93%     
==========================================
  Files           7        8       +1     
  Lines         362      322      -40     
==========================================
- Hits          344      303      -41     
- Misses         18       19       +1     

Impacted Files	Coverage Δ
src/ImageTransformations.jl	83.33% <ø> (ø)
src/resizing.jl	98.78% <ø> (ø)
src/compat.jl	83.33% <83.33%> (ø)
src/autorange.jl	91.83% <100.00%> (-0.48%)	⬇️
src/interpolations.jl	70.83% <100.00%> (-13.95%)	⬇️
src/invwarpedview.jl	97.14% <100.00%> (-0.59%)	⬇️
src/warp.jl	100.00% <100.00%> (ø)
src/warpedview.jl	85.71% <100.00%> (-4.61%)	⬇️

---

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 9257562...3fe10ed. Read the comment docs.

[johnnychen94]

I took the liberty to rewrite the documentation in e0ac5cb and I think this PR is done. It's already quite big so I hope to continue the work in future PRs.

@andrew-saydjari do you want to take a look at the docs change and comment? For example, if this makes things clearer, or makes it less readable.

You can only read that commit difference by clicking it.

[andrew-saydjari]

The documentation changes seem fine to me and are, if anything, clearer. If I recall correctly, Linear() and BSpline(Linear()) are not actually identical if passed to method. There is a subtle difference that Linear() where the transformation does nothing means that the result is === the input, but I don't think that is true for BSpline(Linear()) (only ==). It is just how the different versions of box_extrapolation work. It might be worth mentioning in the docstrings if true.

[johnnychen94]

Nice catch! I totally forget about this difference until you mentioned it.

Internally, box_interpolation skips a so-called "prefilter" step (which allocates new memory) when creating BSplineInterpolation for Linear/Constant), which is why it becomes === when you pass Linear() and == when you pass BSpline(Linear()).

# https://github.com/JuliaMath/Interpolations.jl/blob/e316c736a1369ccf0acd7ffea73af83172c07d34/src/b-splines/b-splines.jl#L159-L162
function interpolate(::Type{TWeights}, ::Type{TC}, A, it::IT) where {TWeights,TC,IT<:DimSpec{BSpline}}
    Apad = prefilter(TWeights, TC, A, it)
    BSplineInterpolation(TWeights, Apad, it, axes(A))
end

I'm not interpolation experts so I don't know whether this "prefilter" step would do to our warp behavior. According to Interpolations documentation of Linear:

Linear b-splines are naturally interpolating, and require no prefiltering; there is therefore no need for boundary conditions to be provided.

Given that all warp operations allocate memory regardless of the interpolation method passed to it, and given that box_interpolation is an internal method,
I intend to take this as an implementation detail that users of warp/imresize/imrotate do not need to be aware of.

---

The only difference is perhaps the performance:

img = testimage("cameraman")

imrotate(img, 0.3; method=Linear()) == imrotate(img, 0.3; method=BSpline(Linear())) # true

@btime imrotate($img, 0.3; method=Linear()); # 3.479 ms (21 allocations: 403.88 KiB)
@btime imrotate($img, 0.3; method=BSpline(Linear())); # 3.491 ms (23 allocations: 660.02 KiB)

I believe we could just do the same optimization to BSpline(Linear()) and BSpline(Constant()) instead of documenting the difference. (#123)

[johnnychen94]

I think this PR is finally done, I'll do a rebase and then merge this.