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

introduce ImageBinarization.jl and ImageContrastAdjustment.jl into Images.jl #841

Closed
johnnychen94 opened this issue Nov 28, 2019 · 7 comments
Assignees
Milestone

Comments

@johnnychen94
Copy link
Member

johnnychen94 commented Nov 28, 2019

A large part of legacy codes in Images.jl (exposure.jl and algorithm.jl)are for histogram and binarization. Since ImageBinarization.jl becomes stable now, we could @reexport ImageBinarization and do some pruning work.

It's quite difficult to determine which part of codes are legacy and can be safely deleted, this issue is used to track the list and make discussions.

Some functions I've identified (and there're more):

  • otsu_threshold in favor of ImageBinarization.Otsu
  • yen_threshold in favor of ImageBinarization.Yen
  • imhist, build_histogram in favor of ImageContrastAdjustment.build_histogram
  • adjust_histogram, adjust_histogram! in favor of ImageContrastAdjustment.adjust_histogram

@zygmuntszpak it would be great if you can give some help 😄

@zygmuntszpak
Copy link
Member

I would like to implement a few more algorithms in both packages and then perhaps submit these packages to https://joss.theoj.org/ with at least, @johnnychen94 and @timholy as coauthors. Would that pose a problem?

@zygmuntszpak
Copy link
Member

If we re-export the contrast adjustment functionality from ImageContrastAdjustment, do we replicate the tests that are in the ImageContrastAdjustment package in the Images package? Or do we want some simpler tests that just verify that the API is callable?

@johnnychen94
Copy link
Member Author

johnnychen94 commented Jan 8, 2020

do we replicate the tests that are in the ImageContrastAdjustment package in the Images package?

Regardless of CI pressure, the only problem is those tests (if hosted in Images.jl) are very likely to be outdated soon. Not only tests, but documentation also has the same problem. And the current status of Images.jl as far as I've seen is to do whatever choice here (or even without tests here); many test codes in Images.jl is becoming legacy.

Or do we want some simpler tests that just verify that the API is callable?

This can be a good choice and I think this is what we can promise so far without too many efforts. (P.S. The current API we use in ImageBinarization doesn't take executing context (e.g., multi-threads, GPU/CPU) into consideration yet.)

As complementary to this option, I'm trying to set up a "syncing" functionality using the power of git submodules in #843 to tackle the same issue for documentation, and if that works well I'm planning to do the same thing for tests.

@zygmuntszpak
Copy link
Member

Given that there has been a burst of activity focused on cleaning up the codebase, I would like to carve out some time to finish introducing ImageContrastAdjustment, ImageBinarization and ideally also ImageEdgeDetection into Images.

Both the ImageContrastAdjustment and ImageEdgeDetection package would like to use the Percentile type which currently is defined in Images. What would be a good place to move that definition of Percentile (out of Images). Would ImageCore be suitable?

@johnnychen94
Copy link
Member Author

johnnychen94 commented Sep 11, 2021

ImageCore sounds good to me since Percentile is only a very thin type without much design on its arithmetic properties. If we want to complicate the implementation of Percentile then it's better to start one more package for this, but I guess we don't need this?

@zygmuntszpak
Copy link
Member

It is indeed a very thin type. Under the hood, I would just dispatch to StatsBase.percentile in the actual packages.

@timholy
Copy link
Member

timholy commented Jul 23, 2023

Closed by #1030

@timholy timholy closed this as completed Jul 23, 2023
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

3 participants