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

Added support for binary images in is_low_contrast to fix imshow of binary images #5247

Closed
wants to merge 3 commits into from

Conversation

MadsDyrmann
Copy link

@MadsDyrmann MadsDyrmann commented Feb 23, 2021

Description

imshow stopped working when plotting boolean images, since np.percentile no longer support boolean arrays. This PR makes sure that is_low_contrast does not fail when the input is a boolean array.

This is a small test-script that would fail before this change:

import numpy as np
from skimage.io import imshow

im = np.ones((10,10), dtype=np.bool)

imshow(im)

Checklist

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.

@pep8speaks
Copy link

pep8speaks commented Feb 23, 2021

Hello @MadsDyrmann! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 651:39: E712 comparison to False should be 'if cond is False:' or 'if not cond:'

Comment last updated at 2021-02-23 16:44:59 UTC

@MadsDyrmann MadsDyrmann changed the title Added support for binary images Added support for binary images in is_low_contrast to fix imshow of binary images Feb 23, 2021
@jni
Copy link
Member

jni commented Feb 24, 2021

Thanks @MadsDyrmann!

Very happy to merge this fix. Do you want to add a test to make sure it doesn't break in the future?

Having said all this, in the medium term (~1y) we aim to pull out the matplotlib dependency and make sure the focus of scikit-image is on image processing, not display or IO. So you should probably start migrating your downstream code to start using plt.imshow directly.

@mkcor
Copy link
Member

mkcor commented Feb 24, 2021

So you should probably start migrating your downstream code to start using plt.imshow directly.

May I recommend plotly.express.imshow as an alternative? With plt.imshow I ran into the issue of being unable to specify value ranges for multichannel data. See this gallery example for basic px.imshow usage.

@jni
Copy link
Member

jni commented Feb 25, 2021

@mkcor well, plotly plots it in a web browser, which some people are allergic to—cough cough cough. 😜

@mkcor
Copy link
Member

mkcor commented Feb 25, 2021

@jni just an alternative... 😜

@@ -647,6 +647,10 @@ def is_low_contrast(image, fraction_threshold=0.05, lower_percentile=1,
if image.shape[2] == 3:
image = rgb2gray(image)

if image.dtype == bool:
return image.any() and (image == False).any()

Copy link
Member

Choose a reason for hiding this comment

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

Maybe I'm missing something, but if a boolean image has both True and False values, then the function should return False, shouldn't it? I have the impression that the returned value should be

Suggested change
return not(image.any() and (image == False).any())

Copy link
Member

Choose a reason for hiding this comment

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

anyway a test (as @jni suggested) could either prove me that I'm wrong :-) or check that the behaviour is as expected

Copy link
Member

Choose a reason for hiding this comment

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

When image.dtype == bool, is_low_contrast is True when the image is constant: all filled with True or with False, ie when False and True are not both in image.

So I think that you are right @emmanuelle!

BTW, I think that this test must take place at the beginning of the function, even before checking for image dimensions, because rgb2gray will convert the image to floating point...

@emmanuelle
Copy link
Member

@MadsDyrmann are the different suggestions clear for you? Thanks again for the PR!

@grlee77 grlee77 added type: bug 🩹 type: Bug fix Fixes unexpected or incorrect behavior and removed type: bug labels Mar 11, 2021
@mkcor
Copy link
Member

mkcor commented Apr 13, 2021

Hi @MadsDyrmann,

To add a test, edit file skimage/exposure/tests/test_exposure.py: Line 749, right after test function test_is_low_contrast(), you can define another test, say, test_is_low_contrast_bool_image() and use the example you came up with, im = np.ones((10,10), dtype=np.bool).

Hope that helps!

@alexdesiqueira
Copy link
Member

Hi everyone,
@MadsDyrmann, would you like to continue working on this PR? We could help you finishing it, if you'd prefer.
Thank you very much for your help so far!

@alexdesiqueira alexdesiqueira added this to the 0.19 milestone May 16, 2021
@alexdesiqueira alexdesiqueira linked an issue May 16, 2021 that may be closed by this pull request
@grlee77
Copy link
Contributor

grlee77 commented Jul 8, 2021

@MadsDyrmann, I have resumed this PR in #5467 so am closing this one as outdated.

Please confirm your name and email address either here or in that PR if you want to be attributed as a co-author on the commits.

@grlee77 grlee77 closed this Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🩹 type: Bug fix Fixes unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

imshow does not work on binary images
8 participants