search

JuliaImages / ImageBinarization.jl

A Julia package of algorithms for analyzing images and automatically binarizing them into background and foreground.
MIT License
35 stars 8 forks source link

dispatch binarize on HistogramThresholding.ThresholdAlgorithm #67

Closed johnnychen94 closed 3 years ago

johnnychen94 commented 4 years ago

It's a bit awkward that two closely related packages have name conflicts:

julia> using HistogramThresholding

julia> using ImageBinarization
[ Info: Precompiling ImageBinarization [cbc4b850-ae4b-5111-9e64-df94c024a13d]

julia> Yen
WARNING: both ImageBinarization and HistogramThresholding export "Yen"; uses of it in module Main must be qualified
ERROR: UndefVarError: Yen not defined

A simple and intuitive solution to this is to reuse the type HistogramThresholding.Yen and directly dispatch binarize on it. But this requires some code organization changes and it may have other unexpected side effects.

zygmuntszpak commented 4 years ago

Yes, I guess it didn't occur to me at the time that one would want to load both packages in the same problem domain.

I'm not sure how to resolve the conflict of the AbstractImageBinarizationAlgorithm type which makes sense in ImageBinarization, and the AbstractHistogramThresholding type (currently just called ThresholdAlgorithm) which makes sense for HistogramThresholding.

Is the name conflict issue coming up in the Images ecosystem for when you try to replace otsu_threshold ? What if we just define a find_threshold for ImageBinarization which essentially wraps the HistogramThresholding implementation?

johnnychen94 commented 4 years ago

Is the name conflict issue coming up in the Images ecosystem for when you try to replace otsu_threshold?

Yes, it turns out that once we reexport any one of these two packages in Images.jl, this issue raises.

I'm not sure how to resolve the conflict of the AbstractImageBinarizationAlgorithm type which makes sense in ImageBinarization, and the AbstractHistogramThresholding type (currently just called ThresholdAlgorithm) which makes sense for HistogramThresholding.

I was thinking of this, too. Will try it and see what possible side effects would it be after this semester.

What if we just define a find_threshold for ImageBinarization which essentially wraps the HistogramThresholding implementation?

In that case, you're encouraging common users to not use HistogramThresholding anymore since find_threshold is the only function there, and if so, it would be better to merge HistogramThresholding as a sub package of ImageBinarization, as @timholy suggested in https://github.com/JuliaImages/Images.jl/issues/898