Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imadjustintensity deprecation suggests adjust_histogram which gives different results #894

Closed
rdeits opened this issue Jun 5, 2020 · 4 comments

Comments

@rdeits
Copy link
Collaborator

rdeits commented Jun 5, 2020

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
Copy link
Collaborator Author

rdeits commented Jun 5, 2020

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
Copy link
Member

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. JuliaImages/ImageContrastAdjustment.jl#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
Copy link
Member

johnnychen94 commented Jun 8, 2020

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
Copy link
Contributor

I have a dangling issue (JuliaImages/ImageContrastAdjustment.jl#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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants