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

State Bradley threshold equivalence in Niblack docstring #3891

Merged
merged 3 commits into from May 15, 2019

Conversation

alexdesiqueira
Copy link
Member

Description

Some info on Bradley and Niblack's thresholds equivalence (see #3877 @jni comments).

Checklist

- [ ] Docstrings for all functions
- [ ] Gallery example in ./doc/examples (new features only)
- [ ] Benchmark in ./benchmarks, if your changes aren't covered by an
existing benchmark

- [ ] Unit tests
- [ ] Clean style in the spirit of PEP8

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.
  • Consider backporting the PR with @meeseeksdev backport to v0.14.x

@alexdesiqueira alexdesiqueira requested review from jni and stefanv May 7, 2019 17:44
@@ -902,10 +902,17 @@ def threshold_niblack(image, window_size=15, k=0.2):
-----
This algorithm is originally designed for text recognition.

The Bradley threshold is a particular case of the Niblack
one, being equivalent to
>>> threshold_niblack(image, k=0) * (1-q)
Copy link
Member

Choose a reason for hiding this comment

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

(a) I think the syntax might require some spacing for proper formatting in the docs:

Suggested change
>>> threshold_niblack(image, k=0) * (1-q)
>>> q = 0.95
>>> t_brad = threshold_niblack(image, k=0) * (1-q)

(b) I used quantile in my explanation because I think they are to percentiles what radians are to degrees, but based on your previous PR I imagine they use percentiles. Therefore I presume they used q = 1. And further, no, they must have used q = 0, because otherwise it reduces to 0 everywhere. (ie they used (1-q) = 1.

Other than those two comments, the PR looks good to me, thanks! =)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey Juan,
now I'm kinda confused. 😅
Yep, they use percentiles. Their algorithm shows (page 4):

20: if (in[i, j) * count) <= (sum * (100-t)/100) then

Then Bradley's should be equal to threshold_niblack() with k=0 only, as before? 😄

Copy link
Member

Choose a reason for hiding this comment

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

Well if t = 100, that number is equal to 0! So someone made a mistake somewhere. =)

Copy link
Member

Choose a reason for hiding this comment

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

Also, (100-t)/100 is equal to (1-q) where q=t/100, ie quantile is the percentile divided by 100.

Copy link
Member Author

Choose a reason for hiding this comment

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

someone made a mistake somewhere. =)

It was me, of course. 😄 Let's try to rewrite this then...

@alexdesiqueira alexdesiqueira requested a review from jni May 8, 2019 17:52
Copy link
Member

@sciunto sciunto left a comment

Choose a reason for hiding this comment

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

A minor comment and I'm ok with this one.

Is there a testcase that checks the equivalence?

References
----------
.. [1] Niblack, W (1986), An introduction to Digital Image
Processing, Prentice-Hall.
.. [2] D. Bradley and G. Roth, "Adaptive thresholding using Integral Image"
Journal of Graphics Tools 12(2), pp. 13-21, 2007.
Copy link
Member

Choose a reason for hiding this comment

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

Please, append :DOI:10.1080/2151237X.2007.10129236

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

The Bradley threshold is a particular case of the Niblack
one, being equivalent to

>>> threshold_niblack(image, k=0)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> threshold_niblack(image, k=0)
>>> q = 1
>>> bradley_t = threshold_niblack(image, k=0) * q
for some value ``q``. By default, Bradley and Roth use ``q=1``.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thanks for your help!

@jni jni changed the title Stating Bradley and Niblack threshold equivalence. State Bradley threshold equivalence in Niblack docstring May 15, 2019
@stefanv stefanv merged commit e6769c5 into scikit-image:master May 15, 2019
@soupault
Copy link
Member

This PR breaks the doctests, see https://travis-ci.org/scikit-image/scikit-image/jobs/532551815#L2615.

@soupault soupault mentioned this pull request May 17, 2019
9 tasks
@jni
Copy link
Member

jni commented May 17, 2019

Dang, I remembered to define q but not image. 🤦‍♂️

@jni jni mentioned this pull request May 17, 2019
9 tasks
@alexdesiqueira alexdesiqueira deleted the niblack_doc branch May 27, 2019 16:39
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.

None yet

6 participants