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

Documentation for color images in histogram matching #3865

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Apr 25, 2019

Description

cleaning unused variables and adding a note about using different colour space than RGB

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.
  • Consider backporting the PR with @meeseeksdev backport to v0.14.x

@pep8speaks
Copy link

pep8speaks commented Apr 25, 2019

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

Line 27:80: E501 line too long (80 > 79 characters)
Line 57:80: E501 line too long (81 > 79 characters)
Line 71:80: E501 line too long (82 > 79 characters)

Comment last updated at 2019-04-27 14:59:53 UTC

@hmaarrfk
Copy link
Member

sigh, sphinx broke things for you.....

@Borda
Copy link
Contributor Author

Borda commented Apr 25, 2019

@hmaarrfk I know, but it came from the actual master... :-/

hmaarrfk and others added 3 commits April 26, 2019 01:27
Co-Authored-By: Borda <Borda@users.noreply.github.com>
Co-Authored-By: Borda <Borda@users.noreply.github.com>
Co-Authored-By: Borda <Borda@users.noreply.github.com>
@hmaarrfk
Copy link
Member

I don't really understand what you are trying to say:

Using RGB image, assume converting input image to some "continuous" color space [2]_.

Can you explain in more words

@Borda
Copy link
Contributor Author

Borda commented Apr 26, 2019

@hmaarrfk can you look at the updated version?

@@ -49,11 +54,9 @@ def match_histograms(image, reference, multichannel=False):
References
----------
.. [1] http://paulbourke.net/miscellaneous/equalisation/
.. [2] https://www.researchgate.net/post/Histogram_matching_for_color_images
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to see a paper rather than a researchgate thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not found any paper about it yet, but see the following example
hist-matching

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found a similar topic here - mapbox/rio-hist#3

Copy link
Member

@sciunto sciunto Apr 29, 2019

Choose a reason for hiding this comment

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

If you add the example, you can remove this link. The example will speak by itself.

@Borda
Copy link
Contributor Author

Borda commented Apr 27, 2019

@sciunto just to clarify, skimage does not support python 2.7 any more, right?

@jni
Copy link
Member

jni commented Apr 27, 2019

@Borda that is correct, you don't need to worry about Py2.7 support and can use syntax for Py3.5+, including the matmul symbol, @.

@hmaarrfk
Copy link
Member

the wording still isn't quite right:

assume to use another colour space

ins't entirely grammatically correct.

You should say something more like:

If multichannel is set to True, images are analyzed on a per channel basis.

This is a fact.

Results for images stored in RGB color space may not behave as expected since transitions are not always smooth in each channel.

General statement that encourages more thought.

Finally, if you want to make this really awesome, I would encourage you to make an example with the images you used in the comment.

@Borda
Copy link
Contributor Author

Borda commented Apr 28, 2019

@hmaarrfk example sounds reasonable, but would I include the image in this repo or download it online when needed? is there any guidence on how to create an example with new/own images?

@hmaarrfk
Copy link
Member

I think the first PR would be to add the image(s) to the dataset of images shipped with scikit-image.

That PR would include:

  1. A license for the image compatible with scikit-image. see discussion about this here: Missing scientific (domain-specific) images and corresponding examples and tutorials #3384
  2. Adding them to the data directory
    https://github.com/scikit-image/scikit-image/blob/master/skimage/data/__init__.py
  3. Ensure it shows up on the gallery (I forget how to do this at the moment).

It seems like you would include two images, the reference, and the source.
Generally here the challenge is to ensure we have the right license for the images.

Not sure how you got that data, but it seems that even lower res images might work to make your point, but high res images would be fine too. I'll push to include your full images.

After you get the data in scikit-image, we can have a more focused discussion on how to improve the example and whatnot.

@sciunto sciunto changed the title cleaning hist. matching Documentation for color images in histogram matching Apr 29, 2019
@Borda
Copy link
Contributor Author

Borda commented May 15, 2019

I need to check the condition of sharing first and try to remember who is the source...

def match_histograms(image, reference, multichannel=False):
"""Adjust an image so that its cumulative histogram matches that of another.

The adjustment is applied separately for each channel.

For color image assume to use another colour space then RGB since the colour
Copy link
Member

Choose a reason for hiding this comment

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

Hey, @Borda, thank you for helping us clarify the documentation; that's always valuable.

I am struggling to understand the new text. Why do you say HSV is "continuous", and RGB is not? I can help wordsmith this a bit, but I don't have a good understanding of the issue at hand.

@@ -48,12 +53,10 @@ def match_histograms(image, reference, multichannel=False):

References
----------
.. [1] http://paulbourke.net/miscellaneous/equalisation/
.. [1] http://paulbourke.net/miscellaneous/equalisation/
Copy link
Member

Choose a reason for hiding this comment

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

Woops, I think all the futzing with this led to incorrect indentation.

@Borda
Copy link
Contributor Author

Borda commented Jul 28, 2019

it seems that with setting multichannel=True the change is not so drastically...
https://github.com/Borda/BIRL/raw/devel/figures/Rat-Kidney_histogram-matching.jpg

@hmaarrfk
Copy link
Member

Can you add the full code?

@Borda
Copy link
Contributor Author

Borda commented Jul 28, 2019

import os
import matplotlib.pyplot as plt
from skimage.io import imread
from skimage.transform import match_histograms
from skimage.color import rgb2hsv, hsv2rgb, rgb2lab, lab2rgb, lab2lch, lch2lab

rgb2lch = lambda im: rgb2lab(lab2lch(im))
lch2rgb = lambda im: lab2rgb(lch2lab(im))

path_imgs = 'https://raw.githubusercontent.com/Borda/BIRL/master/data_images/rat-kidney_/scale-5pc/'
path_img_ref = path_imgs + 'Rat-Kidney_HE.jpg'
path_img_move = path_imgs + 'Rat-Kidney_PanCytokeratin.jpg'

os.system('wget %s' % path_img_ref)
os.system('wget %s' % path_img_move)

img_ref = imread(os.path.basename(path_img_ref))
img_move = imread(os.path.basename(path_img_move))
use_multichannel = True

fig, axarr = plt.subplots(ncols=3, nrows=2, figsize=(12, 6))
axarr[0, 1].set_title('Reference')
axarr[0, 1].imshow(img_ref)
axarr[0, 0].set_title('Source')
axarr[0, 0].imshow(img_move)

axarr[0, 2].set_title('Hist. matching using RGB')
axarr[0, 2].imshow(match_histograms(img_move, img_ref, multichannel=use_multichannel))
axarr[1, 0].set_title('Hist. matching using HSV')
axarr[1, 0].imshow(hsv2rgb(match_histograms(rgb2hsv(img_move), rgb2hsv(img_ref), multichannel=use_multichannel)))
axarr[1, 1].set_title('Hist. matching using LAB')
axarr[1, 1].imshow(lab2rgb(match_histograms(rgb2lab(img_move), rgb2lab(img_ref), multichannel=use_multichannel)))
axarr[1, 2].set_title('Hist. matching using LCH')
axarr[1, 2].imshow(lch2rgb(match_histograms(rgb2lch(img_move), rgb2lch(img_ref), multichannel=use_multichannel)))

Rat-Kidney_histogram-matching

@Borda
Copy link
Contributor Author

Borda commented Jul 29, 2019

@hmaarrfk do you think that it should/could be included or drop this PR?

@hmaarrfk
Copy link
Member

maybe you should rephrase your conclusion, and tell users to use multichannel=True for RGB images?

@Borda
Copy link
Contributor Author

Borda commented Jul 29, 2019

@hmaarrfk Sure and in the doc of the function match_histograms or still add an example?
Rat-Kidney_histogram-matching

@Borda
Copy link
Contributor Author

Borda commented Sep 16, 2019

@hmaarrfk ^^

@hmaarrfk
Copy link
Member

If you can add that to the docs, then just adding it there is fine. Otherwise, this should be a separate example. The documentation will find the example automatically I think

@hmaarrfk
Copy link
Member

I think showing it in an email example gallery will go a long way to helping others. Especially with syntax

Base automatically changed from master to main February 18, 2021 18:23
@grlee77 grlee77 added the 📄 type: Documentation Updates, fixes and additions to documentation label Aug 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 type: Documentation Updates, fixes and additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants