speedup label2rgb using scipy #3344

cmarshak · 2018-08-12T20:17:30Z

Description

This is a pull request related to #3343. It's a speedup to label2rgb using scipy.ndimage.measurements.

I modified the functions used for label2rgb without the overlay keyword arguments.

Checklist

[It's fine to submit PRs which are a work in progress! But before they are merged, all PRs should provide:]

Clean style in the spirit of PEP8
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 - I am not sure how to do this or where to find documentation about this
Unit tests - i did not add more tests, but it successfully ran on the numerous tests for label2rgb intest_colorlabel.py.

[For detailed information on these and other aspects see scikit-image contribution guidelines]

References

[1] scipy.ndimage.measurements

For reviewers

(Don't remove the checklist below.)

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 · 2018-08-12T20:17:32Z

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

In the file skimage/color/colorlabel.py:

Line 114:80: E501 line too long (97 > 79 characters)
Line 246:80: E501 line too long (83 > 79 characters)
Line 248:80: E501 line too long (80 > 79 characters)
Line 262:80: E501 line too long (85 > 79 characters)
Line 297:80: E501 line too long (83 > 79 characters)

Comment last updated at 2019-07-24 18:22:20 UTC

hmaarrfk · 2018-08-12T22:50:38Z

Looks cool! Would you be able to write a benchmark so that we can have a reference point before merging this in?

jni

@cmarshak thank you for spotting this! Yes indeed, I think I found out about labeled_comprehension long after we put this code in scikit-image, so this is a very welcome contribution. However there's still a few improvements needed to include the code. Specifically, the existing code works for multichannel images of any dimension, not just 2D, whereas the new code doesn't. I don't think it should be a big deal to fix though, since ndimage is nd to begin with!

jni · 2018-08-13T01:42:32Z

skimage/color/colorlabel.py

@@ -195,34 +197,87 @@ def _label2rgb_overlay(label, image=None, colors=None, alpha=0.3,
    return result


-def _label2rgb_avg(label_field, image, bg_label=0, bg_color=(0, 0, 0)):
+def _apply_2d_func_to_image(func, image):


I don't want to hardcode the fact that the image is 2D or 2D+c. This function makes sense for 3D images (such as 3D segmentations in microscopy).

You could have multichannel=False as a keyword(-only) argument that determines whether to do apply func to the whole array or to the channels (last dim) separately. In that case, I think we already have such a function in adapt_rgb, although a better name for this would be apply_per_channel.

Thank you for your feedback.

I believe adapt_rgb looks at the shape of the image to adapt a filter or func. I created a function called _apply_per_channel per your suggestion.

Additionally, I added the keyword to make this adaptable in both cases.

jni · 2018-08-13T01:43:52Z

skimage/color/colorlabel.py

+    func : callable
+        A function that takes an array to another array of the same shape.
+    image : array
+        A multichannel image


This is not currently accurate.

jni · 2018-08-13T01:47:16Z

skimage/color/colorlabel.py

@@ -1,6 +1,8 @@
 import itertools

 import numpy as np
+from scipy.ndimage import measurements
+from scipy.ndimage import find_objects


In the rest of the library we use from scipy import ndimage as ndi. I would prefer to use the same convention here, as it makes the code easier to follow once you are familiar with that convention.

jni · 2018-08-13T01:48:11Z

skimage/color/colorlabel.py

+    ----------
+    band : 2d array, shape ``label_field.shape``
+        A multichannel image of the same spatial shape as `label_field`.
+    bg_label : int, optional


This parameter list appears incorrect.

cmarshak · 2018-08-14T00:40:02Z

Thanks for all the helpful feedback. I would be happy to make these improvements, but just started traveling and will not be available to do this for a few weeks. If that works, I can do this when I return. I am sorry for any hold up.

Also, all the comments seem pretty straightforward and will probably have to write some tests to make sure that they are covered easily. However, I am not sure how benchmarks for this project work so will need some references for this.

I really value all the great work you are each doing and allowing me to be involved even minimally. skimage has been very, very helpful for me in my actual work.

jni · 2018-08-14T00:49:00Z

Enjoy your travels! Depending on priorities, we may complete your PR for you, or, more likely, this PR will be waiting for your return. (Especially since the functionality is there, just slow.) Happy to hear you found it useful and we will be very happy to have this contribution and many future ones! =)

cmarshak · 2018-09-28T19:47:52Z

Glad I can still contribute this.

Would you be able to write a benchmark so that we can have a reference point before merging this in?

I am looking at the asv docs and this skimage example. I am assuming that I can adapt this (gist)[https://gist.github.com/cmarshak/ff9a2786d804e38d7c4bc43122509226]. Would I create a separate pull request for this?

jni · 2018-09-29T02:30:37Z

@cmarshak you can append the benchmark to this PR, and post the results of the benchmark (using asv dev, not asv run), before and after the PR changes, as a comment on this thread.

You still need to remove the image.ndim check so that this works for 3D and higher multichannel images.

Thanks for picking this up again!

sciunto · 2019-07-17T06:55:16Z

@cmarshak What's the status of this? Do you still have time to work on this?

For information, #4016 is a PR to provide a how to on asv.

cmarshak · 2019-07-24T18:22:00Z

Yes, I forgot about this - I can work on this in a few weeks when I return from travel. Getting set up with asv slowed me down and then I lost track.

cmarshak · 2019-07-24T18:22:39Z

Whoops - I closed and commented.

soupault · 2019-12-28T15:06:48Z

I'd recommend to have #3015 merged first.

mkcor · 2022-08-24T10:05:12Z

closes #3343

mkcor · 2022-08-24T10:05:49Z

skimage/color/colorlabel.py

+    """
+    Apply a function to each channel of an image.


Suggested change

"""

Apply a function to each channel of an image.

"""Apply a function to each channel of an image.

mkcor · 2022-08-24T10:06:55Z

skimage/color/colorlabel.py

+    """
+    Aggregates pixel values with respect to


Suggested change

"""

Aggregates pixel values with respect to

"""Aggregate pixel values with respect to

Reference: https://peps.python.org/pep-0257/#multi-line-docstrings

mkcor · 2022-08-24T10:07:27Z

skimage/color/colorlabel.py

+        An array with the same shape and type as `image` and `label_field`
+    """
+    if label_field.shape != image.shape:
+        raise ValueError('label_field and image must have the same shape as image')


Suggested change

raise ValueError('label_field and image must have the same shape as image')

raise ValueError('label_field must have the same shape as image')

mkcor · 2022-08-24T10:08:21Z

skimage/color/colorlabel.py

+    return out
+
+
+def _label2mean(label_field, image, bg_label=None, bg_color=None, multichannel=True):
    """Visualise each segment in `label_field` with its mean color in `image`.


Suggested change

"""Visualise each segment in `label_field` with its mean color in `image`.

"""Visualize each segment in `label_field` with its mean color in `image`.

(convention)

mkcor · 2022-08-24T10:14:21Z

skimage/color/colorlabel.py

+    return out
+
+
+def _label2mean(label_field, image, bg_label=None, bg_color=None, multichannel=True):


Suggested change

def _label2mean(label_field, image, bg_label=None, bg_color=None, multichannel=True):

def _label2mean(label_field, image, bg_label=None, bg_color=None, channel_axis=-1):

It would be great to update this parameter!

References:

https://github.com/scikit-image/scikit-image/blob/main/doc/release/release_0.19.rst#announcement-scikit-image-0190

https://scikit-image.org/docs/stable/user_guide/numpy_images.html?highlight=channel_axis#color-images

scikit-image/skimage/color/colorconv.py

Line 63 in b566789

def convert_colorspace(arr, fromspace, tospace, *, channel_axis=-1):

speedup label2rgb using scipy

a326c2b

cmarshak mentioned this pull request Aug 12, 2018

Using Scipy.ndimage to speedup label2rgb #3343

Open

linting

0ad7460

This comment has been minimized.

Sign in to view

jni requested changes Aug 13, 2018

View reviewed changes

soupault added the 📈 type: Performance label Aug 17, 2018

soupault added this to the 0.15 milestone Aug 17, 2018

soupault self-assigned this Aug 17, 2018

cmarshak added 4 commits September 14, 2018 16:34

fixed comments from jni

61af3f7

added label comprehension to color to label

e8830a1

Merge branch 'master' into speedup-label2rgb-with-scipy

deeb21a

updated channel_by_channel name

8758839

soupault modified the milestones: 0.15, 0.16 Apr 20, 2019

sciunto modified the milestones: 0.16, 0.17 Jul 17, 2019

cmarshak closed this Jul 24, 2019

cmarshak reopened this Jul 24, 2019

Base automatically changed from master to main February 18, 2021 18:23

mkcor mentioned this pull request Feb 22, 2021

2021's calendar of community management #5169

Closed

grlee77 mentioned this pull request Mar 27, 2022

WIP: Turn off relabelling in label2rgb #3015

Open

7 tasks

jarrodmillman modified the milestones: 0.17, 0.20 Jun 4, 2022

mkcor reviewed Aug 24, 2022

View reviewed changes

lagru mentioned this pull request Aug 28, 2022

2022's calendar of community management #6165

Closed

lagru modified the milestone: 0.20 Oct 25, 2022

stefanv removed this from the 0.20 milestone Jan 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

speedup label2rgb using scipy #3344

speedup label2rgb using scipy #3344

cmarshak commented Aug 12, 2018

pep8speaks commented Aug 12, 2018 •

edited

This comment has been minimized.

hmaarrfk commented Aug 12, 2018

jni left a comment

jni Aug 13, 2018

jni Aug 13, 2018

cmarshak Sep 28, 2018 •

edited

jni Aug 13, 2018

jni Aug 13, 2018

jni Aug 13, 2018

cmarshak commented Aug 14, 2018

jni commented Aug 14, 2018

cmarshak commented Sep 28, 2018

jni commented Sep 29, 2018

sciunto commented Jul 17, 2019

cmarshak commented Jul 24, 2019

cmarshak commented Jul 24, 2019

soupault commented Dec 28, 2019

mkcor commented Aug 24, 2022

mkcor Aug 24, 2022

mkcor Aug 24, 2022

mkcor Aug 24, 2022

mkcor Aug 24, 2022

mkcor Aug 24, 2022

	"""
	Apply a function to each channel of an image.
	"""Apply a function to each channel of an image.

	"""
	Aggregates pixel values with respect to
	"""Aggregate pixel values with respect to

	raise ValueError('label_field and image must have the same shape as image')
	raise ValueError('label_field must have the same shape as image')

	"""Visualise each segment in `label_field` with its mean color in `image`.
	"""Visualize each segment in `label_field` with its mean color in `image`.

		return out


		def _label2mean(label_field, image, bg_label=None, bg_color=None, multichannel=True):

speedup label2rgb using scipy #3344

Are you sure you want to change the base?

speedup label2rgb using scipy #3344

Conversation

cmarshak commented Aug 12, 2018

Description

Checklist

References

For reviewers

pep8speaks commented Aug 12, 2018 • edited

Comment last updated at 2019-07-24 18:22:20 UTC

This comment has been minimized.

hmaarrfk commented Aug 12, 2018

jni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmarshak Sep 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmarshak commented Aug 14, 2018

jni commented Aug 14, 2018

cmarshak commented Sep 28, 2018

jni commented Sep 29, 2018

sciunto commented Jul 17, 2019

cmarshak commented Jul 24, 2019

cmarshak commented Jul 24, 2019

soupault commented Dec 28, 2019

mkcor commented Aug 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Aug 12, 2018 •

edited

cmarshak Sep 28, 2018 •

edited