Refactor invertible mapping functions?

[stillyslalom]

I've been mentally bikeshedding the linear contrast stretching interface discussed in this thread. I'd planned to just add a few methods to scaleminmax in ImageCore, but I think the functionality fits better in this sub-package.

I think adjust_histogram is a misnomer for the functions within this module that don't involve the construction or adjustment of an image histogram. Looking at scikit-image's exposure module, a distinction is made between histogram-based and non-histogram-based intensity adjustment functions. The same is true for Matlab's image processing toolbox.

With that in mind, I'd like to propose that the non-histogram-based algorithms be moved to a new category AbstractIntensityMappingAlgorithm <: AbstractImageFilter. This would include LinearStretching, ConstrastStretching, GammaCorrection, and other non-binning, invertible maps.

[zygmuntszpak]

Thanks you for your suggestion. Please could you clarify a few more details. Do you have in mind a function

adjust_intensity(img, algo::AbstractIntensityMappingAlgorithm)

so that one writes things such as:

adjust_intensity(img, LinearStretching(...))
adjust_intensity(img, ContrastStretching(...))
adjust_intensity(img, GammaCorrection(...))

In #33 it seems that you want to adjust_intensity to be an alias for the LinearStretching algorithm?

If I understood you correctly, you want to introduce a standalone function as an alias for adjust_intensity(img, LinearStretching(...)) with some sensible default values, and additional useful dispatch behavior. For example, something like

span_contrast(img, Percentiles(1, 99))

[stillyslalom]

The refactored algos would be dispatched through a top-level adjust_intensity function, with LinearStretching(extrema) set as its default argument. If the contrast-stretching functionality is easily accessible through adjust_intensity, there doesn't seem to be any need for a standalone alias.

But this is a better discussion for #33 - I'm trying to separate the bikesheddy interface design of #33 from the type hierarchy question raised in this issue.

[zygmuntszpak]

I think your suggestion of introducing AbstractIntensityMappingAlgorithm <: AbstractImageFilter and associating LinearStretching, ConstrastStretching, GammaCorrection with that type makes sense.

[johnnychen94]

Having a convenient method adjust_histogram(img) with a default algorithm makes sense to me. It can be a fixed one such as LinearStretching, or it could be adaptive:

adjust_histogram(img) = adjust_histogram(img, best_algorithm_and_parameter(img))

Moving/re-implementing scaleminmax things from ImageCore here seems a good idea to me, but I'm too busy recently to do that myself. @stillyslalom if you can do it then I'm very happy to review it.

I'm not rejecting it, but introducing a new name adjust_intensity for pure conceptual and nominal correctness is not so appealing to me. Recently we're making a lot of deprecations and API changes, I would like to make them as few as possible unless there's a real need. We could leave this for future API discussion and deprecate all those in a batch.

[stillyslalom]

I use Images.jl to process quantitative experimental data, and I would rather not have the default option for intensity adjustment be 'best guess!'. The default should be stable and sensible, and if a user is feeling lucky, they should have to choose that explicitly. Imagine a case where the user intensity-adjusts a time series of images, only to find that a different algorithm has been used on each frame!

I'm of the opinion that there's no harm in another minor v0.x release to reinforce a solid interface and type hierarchy, since once we hit 1.0, it becomes much more challenging to revise the API. @timholy, thoughts?

[johnnychen94]

My point about adaptiveness is, if you're too lazy to even choose an algorithm, then by default this package makes a guess for you; if you do care about the algorithm, then write it down explicitly.

TBH, I don't really think this is inconvenient in practice.

adjust_histogram.(imgs, Ref(LinearStretching()))

Said that, I don't have a strong opinion on whether it should be adaptive or fixed(adaptiveness=0).

[timholy]

I like the idea of renaming to adjust_intensity; I'm with @stillyslalom that the name adjust_histogram shouldn't lie about what it's doing, and in any event seems more specific and less clear than optimal:

help?> adjust_histogram
search: adjust_histogram adjust_histogram!

  adjust_histogram([T::Type,] img, f::AbstractHistogramAdjustmentAlgorithm, args...; kwargs...)

  Adjust histogram of img using algorithm f.

This summary could confuse naive readers in several ways:

• it almost implies that the algorithm operates in-place, but
• to a naive user, "the histogram of img" sounds like some external object (or image metadata?)
• it doesn't clarify that what actually happens is that the intensities in the output are different from the input, but the image is otherwise "the same."

A more approachable one-line summary might be something like "Compute an intensity-adjusted version of img using algorithm f"