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

[WIP] refactor codebase with functor APIs #29

Merged
merged 53 commits into from
Jul 31, 2019
Merged

[WIP] refactor codebase with functor APIs #29

merged 53 commits into from
Jul 31, 2019

Conversation

johnnychen94
Copy link
Member

@johnnychen94 johnnychen94 commented Jul 21, 2019

I found filter specification greatly reduces the complexity of code structure and improves the style consistency when I played with ImageQualityIndexes and ImageNoise. Check my GSoC 2019 blog The principles of Images.jl: Part I as an example and illustration. Also, see Proposal: add abstract types ImageAlgorithm and ImageFilter for a unified API that enables extensibility without loss of API consistency.

To make this PR easy to review, the first eight commits serve as a start point, future commits are submitted in the form of PRs.

TODO:

* replace `BinarizationAlgorithm` with `AbstractImageBinarizationAlgorithm`
* import `binarize` and `binarize!` from `BinarizationAPI`
* move concrete algorithms struct definitions to seperated files

This is a intermediate commit as a start point for the following code refactor, i.e.,
moving implementations to concrete functors
@codecov
Copy link

codecov bot commented Jul 22, 2019

Codecov Report

Merging #29 into master will decrease coverage by 23.92%.
The diff coverage is 95.95%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #29       +/-   ##
===========================================
- Coverage   94.47%   70.55%   -23.93%     
===========================================
  Files          16       20        +4     
  Lines         163      180       +17     
===========================================
- Hits          154      127       -27     
- Misses          9       53       +44
Impacted Files Coverage Δ
src/sauvola.jl 0% <ø> (-93.75%) ⬇️
src/niblack.jl 0% <ø> (-93.75%) ⬇️
src/ImageBinarization.jl 100% <ø> (ø) ⬆️
src/polysegment.jl 0% <ø> (-91.67%) ⬇️
src/minimum_error.jl 100% <100%> (ø) ⬆️
src/intermodes.jl 100% <100%> (ø) ⬆️
src/otsu.jl 100% <100%> (ø) ⬆️
src/moments.jl 100% <100%> (ø) ⬆️
src/BinarizationAPI/binarize.jl 100% <100%> (ø)
src/unimodal.jl 100% <100%> (ø) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ff0e10...07704cb. Read the comment docs.

* move implementations to AdaptiveThreshold functor

* Rewrite AdaptiveThreshold with CartesianIndices to support n-D images

* update and simplify the docstring

* enhance the test codes and fix several bugs

* add CartetianIndex compat to Julia 1.0

* deprecate `window_size` and `recommend_size`
This is a PR-commit for PR
#29
that refactors the whole codebase to use the new functor API, which
is discussed in more details in
#26
and explained in my GSoC 2019 blog
https://nextjournal.com/johnnychen94/the-principles-of-imagesjl-part-i

Changes:

* move implementations into functor
* enhance the test codes with wider test coverage
This is a PR-commit for PR
#29 that
refactors the whole codebase to use the new functor API, which
is discussed in more details in
#26 and
explained in my [GSoC 2019
blog](https://nextjournal.com/johnnychen94/the-principles-of-imagesjl-part-i)

Changes:

* support `Color3` input
* move implementations into functor
* enhance the test codes with wider test coverage
This is a PR-commit for PR
#29 that
refactors the whole codebase to use the new functor API, which
is discussed in more details in
#26 and
explained in my [GSoC 2019
blog](https://nextjournal.com/johnnychen94/the-principles-of-imagesjl-part-i)

Changes:

* support `Color3` input
* move implementations into functor
* enhance the test codes with wider test coverage
This is a PR-commit for PR
#29 that
refactors the whole codebase to use the new functor API, which is
discussed in more details in
#26 and
explained in my [GSoC 2019
blog](https://nextjournal.com/johnnychen94/the-principles-of-imagesjl-part-i)

Changes:

* support `Color3` input
* move implementations into functor
* enhance the test codes with wider test coverage
This is a PR-commit for PR
#29 that
refactors the whole codebase to use the new functor API, which is
discussed in more details in
#26 and
explained in my [GSoC 2019
blog](https://nextjournal.com/johnnychen94/the-principles-of-imagesjl-part-i)

Changes:

* support `Color3` input
* move implementations into functor
* enhance the test codes with wider test coverage
function binarize(algorithm::AdaptiveThreshold, img::AbstractArray{T,2}) where T <: Gray
s = algorithm.window_size
t = algorithm.percentage
if s < 0 || t < 0 || t > 100
Copy link
Member Author

@johnnychen94 johnnychen94 Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a reminder to myself:

we need to check whether the given window_size is valid, i.e., >=0 for AdaptiveThreshold, Niblack, Sauvola.

@codecov-io
Copy link

Codecov Report

Merging #29 into master will decrease coverage by 23.92%.
The diff coverage is 95.95%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #29       +/-   ##
===========================================
- Coverage   94.47%   70.55%   -23.93%     
===========================================
  Files          16       20        +4     
  Lines         163      180       +17     
===========================================
- Hits          154      127       -27     
- Misses          9       53       +44
Impacted Files Coverage Δ
src/sauvola.jl 0% <ø> (-93.75%) ⬇️
src/niblack.jl 0% <ø> (-93.75%) ⬇️
src/ImageBinarization.jl 100% <ø> (ø) ⬆️
src/polysegment.jl 0% <ø> (-91.67%) ⬇️
src/minimum_error.jl 100% <100%> (ø) ⬆️
src/intermodes.jl 100% <100%> (ø) ⬆️
src/otsu.jl 100% <100%> (ø) ⬆️
src/moments.jl 100% <100%> (ø) ⬆️
src/BinarizationAPI/binarize.jl 100% <100%> (ø)
src/unimodal.jl 100% <100%> (ø) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ff0e10...88202fe. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Jul 29, 2019

Codecov Report

Merging #29 into master will increase coverage by 0.46%.
The diff coverage is 96.69%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
+ Coverage   94.47%   94.94%   +0.46%     
==========================================
  Files          16       20       +4     
  Lines         163      178      +15     
==========================================
+ Hits          154      169      +15     
  Misses          9        9
Impacted Files Coverage Δ
src/util.jl 100% <ø> (ø) ⬆️
src/ImageBinarization.jl 100% <ø> (ø) ⬆️
src/minimum_error.jl 100% <100%> (ø) ⬆️
src/intermodes.jl 100% <100%> (ø) ⬆️
src/otsu.jl 100% <100%> (ø) ⬆️
src/sauvola.jl 94.73% <100%> (+0.98%) ⬆️
src/moments.jl 100% <100%> (ø) ⬆️
src/BinarizationAPI/binarize.jl 100% <100%> (ø)
src/niblack.jl 100% <100%> (+6.25%) ⬆️
src/unimodal.jl 100% <100%> (ø) ⬆️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ff0e10...125c3e4. Read the comment docs.

Otherwise it would return `nothing`
@johnnychen94
Copy link
Member Author

johnnychen94 commented Jul 31, 2019

Regardless of all those dirty and lengthy details in this PR which I believe nobody would like to review, as a summary, this PR does:

  • refactor the codebase using the functor API discussed in Filter specification: an alternative to binarize #26
  • enhance the API by introducing a submodule BinarizationAPI
  • add in-place function binarize!
  • support Color3 inputs
  • add more test codes
  • slightly enhance the documentation

Breaking changes (Deprecated in 0.3):

made window_size of AdaptiveThreshold not an optional argument.

The idea here is to encourage users to use AdaptiveThreshold(img) to infer window_size rather than some fixed integer 32.


Any comments before I merge this PR? @zygmuntszpak

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

Successfully merging this pull request may close these issues.

RFC: a new API that plans algorithm with prior information before binarize Swap arguments?
3 participants