JuliaImages / ImageFiltering.jl

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

Improve (?) deprecation warning in #75 #78

Closed timholy closed 5 years ago

timholy commented 5 years ago

I was going through some old emails and realized none of us had tagged a release containing #75. But then I tested and found the following deprecation warning a bit opaque:

julia> mapwindow!(identity, out, A, (0:1, 0:1), Inner())
┌ Warning: `mapwindow!(f, out, img, window, border, indices=default_imginds(img, window, border))` is deprecated, use `begin
│     mapwindow!(f, out, img, window, border=border, indices=indices)
│ end` instead.
│   caller = mapwindow!(::Function, ::Array{SArray{Tuple{4},Float64,1,4},2}, ::Array{Float64,2}, ::Tuple{UnitRange{Int64},UnitRange{Int64}}, ::Inner{0}) at deprecated.jl:54
└ @ ImageFiltering.MapWindow ./deprecated.jl:54

This PR changes it to

julia> mapwindow!(identity, out, A, (0:1, 0:1), Inner())
┌ Warning: mapwindow!(f, out, img, window, Inner{0}((), ())) is deprecated, use `mapwindow!(f, out, img, window, border=Inner{0}((), ()))` instead.
│   caller = top-level scope at none:0
└ @ Core none:0

which I think is better. (The line numbers are wrong, but that's true for both of them; the second is arguably more realistic.) It's still not perfect---Inner{0}((), ()) is more complicated than Inner() which is what the user typed---but I think it's close enough that people may be able to recognize which argument they should pass as a keyword argument.

What do you think, @jw3126? An improvement, or not?

codecov[bot] commented 5 years ago

Codecov Report

Merging #78 into master will increase coverage by 0.15%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #78      +/-   ##
==========================================
+ Coverage   94.79%   94.94%   +0.15%     
==========================================
  Files           8        8              
  Lines         692      693       +1     
==========================================
+ Hits          656      658       +2     
+ Misses         36       35       -1
Impacted Files Coverage Δ
src/mapwindow.jl 97.87% <ø> (ø) :arrow_up:
src/border.jl 99.02% <0%> (ø) :arrow_up:
src/imfilter.jl 91.02% <0%> (+0.3%) :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 6e21e4f...832e759. Read the comment docs.

jw3126 commented 5 years ago

I agree, the second variant is a bit clearer.

timholy commented 5 years ago

Thanks for the quick review, @jw3126!