JuliaImages / Images.jl

An image library for Julia
http://juliaimages.org/
Other
535 stars 143 forks source link

`imadjustintensity` deprecation suggests `adjust_histogram` which gives different results #894

Closed rdeits closed 3 years ago

rdeits commented 4 years ago

I'm working on updating https://github.com/rdeits/EdgeCameras.jl and I noticed a deprecation warning about imadjustintensity being replaced with adjust_histogram(img, LinearStretching()). However, at least for the data I'm working with, those two methods produce very different results.

Here's a very simple example:

julia> using Images

julia> img = RGB{Float32}[RGB{Float32}(6.024074f-7,-2.481209f-5,-1.2082533f-5), RGB{Float32}(6.815455f-5,6.5870896f-5,7.189392f-5)]
2-element Array{RGB{Float32},1} with eltype RGB{Float32}:
 RGB{Float32}(6.024074f-7,-2.481209f-5,-1.2082533f-5)
 RGB{Float32}(6.815455f-5,6.5870896f-5,7.189392f-5)

julia> imadjustintensity(img)
┌ Warning: `imadjustintensity` will be removed in a future release, please use `adjust_histogram(img, LinearStretching())` instead.
│   caller = top-level scope at REPL[3]:1
└ @ Core REPL[3]:1
2-element Array{RGB{Float32},1} with eltype RGB{Float32}:
 RGB{Float32}(0.26280165f0,0.0f0,0.1316315f0)
 RGB{Float32}(0.9613326f0,0.9377182f0,1.0f0)

julia> adjust_histogram(img, LinearStretching())
2-element Array{RGB{Float32},1} with eltype RGB{Float32}:
 RGB{Float32}(4.22286f-7,0.0f0,0.0f0)
 RGB{Float32}(1.0f0,0.99999857f0,1.0f0)

The new adjust_histogram makes the first pixel nearly black and the second white, while the old imadjustintensity preserved some of the color information.

Is there a way to accurate reproduce the old imadjustintensity behavior with the new API? And can we change the deprecation warning to match?

rdeits commented 4 years ago

Also, how would you feel about making imadjustintensity use the actual built-in deprecation warning system? As it is, the warning happens for every call, while the built-in @deprecate ensures that the warning only happens once. @deprecate also allows a user to do --depwarn=error to immediately find deprecation warnings and fix them in tests.

zygmuntszpak commented 4 years ago

Thanks you for raising the issue. If you had been working with grayscale images, then the direct replacement for

imadjustintensity(img)

would be

adjust_histogram(img, LinearStretching(nothing => (0,1))

and so the deprecation message as it currently stands is misleading. The issue boils down to the fact that

the args to imadjustintensity specify the "from" range, whereas those of the same name to LinearStretching are the "to" range. https://github.com/JuliaImages/ImageContrastAdjustment.jl/issues/27

So by writing

adjust_histogram(img, LinearStretching(nothing => (0,1))

you are saying you want to map extrema(img) to the interval (0,1).

The situation is a little more nuanced for RGB images because the convention used in the ImageContrastAdjustment.jl package is that a color image is always mapped to the YIQ color space. The operation is then applied to the Y-channel before transforming the result back to an RGB image.

The imadjustintensity does not do the conversion to YIQ space and hence yields different results. Replicating the imadjustintensity with adjust_histogram requires a little more effort.

julia> img = RGB{Float32}[RGB{Float32}(6.024074f-7,-2.481209f-5,-1.2082533f-5), RGB{Float32}(6.815455f-5,6.5870896f-5,7.189392f-5)]
2-element Array{RGB{Float32},1} with eltype RGB{Float32}:
 RGB{Float32}(6.024074f-7,-2.481209f-5,-1.2082533f-5)
 RGB{Float32}(6.815455f-5,6.5870896f-5,7.189392f-5)

ls = LinearStretching(extrema(channelview(img)) => nothing);
img2 = copy(img);
map(i -> adjust_histogram!(view(channelview(img2), i, :,:), ls), 1:3);

julia> img2
2-element Array{RGB{Float32},1} with eltype RGB{Float32}:
 RGB{Float32}(0.26280162f0,0.0f0,0.1316315f0)
 RGB{Float32}(0.9613326f0,0.9377182f0,1.0f0) 

A "cleaner" way of achieving what you want is with takemap, e.g.

f = takemap(scaleminmax,img)
julia> f.(img)
2-element Array{RGB{Float32},1} with eltype RGB{Float32}:
 RGB{Float32}(0.26280165f0,0.0f0,0.1316315f0)
 RGB{Float32}(0.9613326f0,0.9377182f0,1.0f0) 

I noticed that the values of your example RGB image actually contain negative values, whereas normally valid values are supposed to lie within the unit interval. If I understood correctly, you were trying to map these values into a valid range with imadjustintensity?

@timholy Is there any particular reason why we went with Base.depwarn instead of @deprecate?

johnnychen94 commented 4 years ago

Is there any particular reason why we went with Base.depwarn instead of @deprecate?

In simple words, @deprecate provides a convenient API that calls Base.depwarn to cover most of the deprecation usage, especially for name changes.

For more complex deprecation that deprecate cannot handle, we need to manually construct a deprecation message with Base.depwarn. For example, in ImageBinarization.jl, the depwarn in binarize involves method dispatching(not very sure if this case can be handled by @deprecate), and that in recommend_size use a custom message.

stillyslalom commented 4 years ago

I have a dangling issue (https://github.com/JuliaImages/ImageContrastAdjustment.jl/issues/32) related to this functionality. It still bothers me that the 'cleaner' solution for contrast stretching requires multiple steps instead of a self-contained function call. Sorry I haven't found time to make the PR yet!