WIP: Turn off relabelling in label2rgb #3015

jni · 2018-04-12T09:51:17Z

Description

I also updated parameter names and deprecated background=-1 as the default. I am currently stating that background=0 will be the default but I could also go for background=None.

This is the first PR with very verbose warning and error messages that we discussed in the mailing list. Comments very welcome.

Also included: a new utility DEFAULT value for use instead of None when deprecating. This makes it easy to distinguish between the use of the DEFAULT value because no value was passed, vs when the user explicitly passes None. I've kept it as simple as possible for now but this could be even more sophisticated, with a DEFAULT object actually holding a default value, rather than having to hardcode it somewhere inside the function. Comments also welcome here!

I still need to add tests for all the new code paths.

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)
Unit tests

References

Closes issue #2674

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.

This warning appears to have been adopted in scikit-image#485 with no discussion at all. imho it is unnecessary: if we want to support negative intensity, we should do it correctly (fixing the intensity), rather than raising a warning and doing nothing (since the function silently succeeds). Refs: scikit-image#485

Can turn off relabelling to consecutive labels before color matching. Addresses issue scikit-image#2674

pep8speaks · 2018-04-12T09:51:20Z

Hello @jni! Thanks for submitting the PR.

In the file skimage/color/colorlabel.py, following are the PEP8 issues :

Line 259:41: E128 continuation line under-indented for visual indent

stefanv · 2018-05-23T06:08:21Z

skimage/_shared/_default_value.py

@@ -0,0 +1,14 @@
+class DefaultValue(object):


Use Ellipsis instead of None?

Good suggestion, 👍

@stefanv @soupault to clarify, you mean, get rid of this object, and just use Ellipsis directly?

As I see it, use Ellipsis instead of 'None' in this class. Your option would work equally well, though

sciunto · 2019-09-22T05:28:32Z

@jni. I postponed your PR to 0.17. Perhaps you can have a quick look to check the status of it? :)

soupault

I think this a good change 👍. Added some points to discuss.

soupault · 2019-12-28T14:46:48Z

skimage/color/colorlabel.py


    Returns
    -------
    result : array of float, shape (M, N, 3)
        The result of blending a cycling colormap (`colors`) for each distinct
        value in `label` with the image, at a certain alpha value.
    """
+    if image is None and kind in ['avg' or 'average']:


Suggested change

if image is None and kind in ['avg' or 'average']:

if image is None and kind in ('avg', 'average'):

soupault · 2019-12-28T14:49:44Z

skimage/color/colorlabel.py

    Returns
    -------
-    result : array of float, shape (M, N, 3)
+    result : array of float, shape (M, N,[[ P,] ...,] 3)


Suggested change

result : array of float, shape (M, N,[[ P,] ...,] 3)

result : array of float, shape (M, N,[ P, [...,]] 3)

soupault · 2019-12-28T14:56:35Z

skimage/color/colorlabel.py

@@ -75,7 +79,9 @@ def _match_label_with_color(label, colors, bg_label, bg_color):


 def label2rgb(label, image=None, colors=None, alpha=0.3,
-              bg_label=-1, bg_color=(0, 0, 0), image_alpha=1, kind='overlay'):
+              background=DEFAULT, background_color=None, image_alpha=1,


If I understood correctly, this DEFAULT thing is introduced only to help in handling such complex deprecation scenarios. I think using it for other purposes would reduce user experience and should not be recommended.
Pehaps, changing the name to UPDATED could better show its temporary nature.

soupault · 2019-12-28T14:57:51Z

skimage/color/colorlabel.py

@@ -75,7 +79,9 @@ def _match_label_with_color(label, colors, bg_label, bg_color):


 def label2rgb(label, image=None, colors=None, alpha=0.3,
-              bg_label=-1, bg_color=(0, 0, 0), image_alpha=1, kind='overlay'):
+              background=DEFAULT, background_color=None, image_alpha=1,
+              kind='overlay', relabel=DEFAULT,


Suggested change

kind='overlay', relabel=DEFAULT,

kind='overlay', *, relabel=DEFAULT,

soupault · 2019-12-28T15:00:07Z

skimage/_shared/_default_value.py

@@ -0,0 +1,14 @@
+class DefaultValue(object):


Good suggestion, 👍

soupault · 2019-12-28T15:01:02Z

skimage/color/colorlabel.py

-    label : array, shape (M, N)
-        Integer array of labels with the same shape as `image`.
-    image : array, shape (M, N, 3), optional
+    label : array, shape (M, N,[[ P,] ...])


Suggested change

label : array, shape (M, N,[[ P,] ...])

label : array, shape (M, N,[ P[, ...]])

soupault · 2019-12-28T15:03:21Z

skimage/color/colorlabel.py

+    label : array, shape (M, N,[[ P,] ...])
+        Integer array of labels with the same shape as `image` (not including
+        channels).
+    image : array, shape (M, N,[[[ P,] ...,] 3]), optional


Suggested change

image : array, shape (M, N,[[[ P,] ...,] 3]), optional

image : array, shape (M, N,[ P,[ ...,][ 3]), optional

jni · 2019-12-29T21:41:48Z

@soupault thanks for the blast from the past review! =) I'll try to pick this one up again soon. Yikes! LOL

IssamLaradji · 2020-04-21T21:59:43Z

any updates on this? this is very useful!

rfezzani · 2020-04-21T22:45:24Z

I don't know if this PR is still relevant (please see #4464).

grlee77 · 2022-03-27T14:07:50Z

I don't know if this PR is still relevant (please see #4464).

A subset of this was done elsewhere (the bg_label default was changed from -1 to 0). I am also not a big fan of using DEFAULT and think the we should use the more recently developed decorators for deprecations, but other items here are still relevant:

The proposed API change from:
bg_label->background
bg_color->background_color
seems reasonable to me and has not been done.

The other changes here are

introduction of a relabel keyword-only argument to label2rgb
more descriptive error message

If there is consensus that we should still work on these changes, I am happy to help update this PR along with another performance-based PR for label2rgb in #3344.

rfezzani · 2022-03-28T07:04:01Z

I'm not a big fan of DEFAULT neither. Otherwise, the other suggestions are fine by me.

jni added 8 commits April 12, 2018 11:35

Make label2rgb nd-compatible

76a56e4

Add more meaningful error message when shapes mismatch

0a784c0

More concise conversion of image to alpha-corrected RGB

8477573

Improve formatting of label2rgb ValueError message

9385aaa

Handle deprecations of bg_label and bg_color

436cb2f

Add relabel argument to label2rgb

6b6ad71

Can turn off relabelling to consecutive labels before color matching. Addresses issue scikit-image#2674

Add default value updates to TODO

3070318

jni mentioned this pull request Apr 13, 2018

Strange behavior of 'label2rgb' function #2674

Open

soupault added the 💪 Work in progress label May 7, 2018

soupault added this to the 0.14 milestone May 7, 2018

stefanv reviewed May 23, 2018

View reviewed changes

soupault modified the milestones: 0.14, 0.14.1 May 29, 2018

soupault self-assigned this Jul 29, 2018

soupault modified the milestones: 0.14.1, 0.15 Aug 20, 2018

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

sciunto modified the milestones: 0.16, 0.17 Sep 22, 2019

soupault reviewed Dec 28, 2019

View reviewed changes

soupault mentioned this pull request Dec 28, 2019

speedup label2rgb using scipy #3344

Open

9 tasks

rfezzani mentioned this pull request Apr 24, 2020

Change label2rgb default background value from -1 to 0 #4614

Merged

8 tasks

sciunto mentioned this pull request Apr 27, 2020

2020's calendar of community management #4486

Closed

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 added the 📜 type: API Involves API change(s) label Mar 27, 2022

grlee77 modified the milestones: 0.17, skimage2 Mar 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Turn off relabelling in label2rgb #3015

WIP: Turn off relabelling in label2rgb #3015

jni commented Apr 12, 2018

pep8speaks commented Apr 12, 2018

stefanv May 23, 2018

soupault Dec 28, 2019

jni Dec 29, 2019

soupault Dec 30, 2019

sciunto commented Sep 22, 2019

soupault left a comment

soupault Dec 28, 2019

soupault Dec 28, 2019

soupault Dec 28, 2019

soupault Dec 28, 2019

soupault Dec 28, 2019

soupault Dec 28, 2019

soupault Dec 28, 2019

jni commented Dec 29, 2019

IssamLaradji commented Apr 21, 2020

rfezzani commented Apr 21, 2020

grlee77 commented Mar 27, 2022 •

edited

rfezzani commented Mar 28, 2022

	if image is None and kind in ['avg' or 'average']:
	if image is None and kind in ('avg', 'average'):

	result : array of float, shape (M, N,[[ P,] ...,] 3)
	result : array of float, shape (M, N,[ P, [...,]] 3)

	kind='overlay', relabel=DEFAULT,
	kind='overlay', *, relabel=DEFAULT,

	label : array, shape (M, N,[[ P,] ...])
	label : array, shape (M, N,[ P[, ...]])

	image : array, shape (M, N,[[[ P,] ...,] 3]), optional
	image : array, shape (M, N,[ P,[ ...,][ 3]), optional

WIP: Turn off relabelling in label2rgb #3015

Are you sure you want to change the base?

WIP: Turn off relabelling in label2rgb #3015

Conversation

jni commented Apr 12, 2018

Description

Checklist

References

For reviewers

pep8speaks commented Apr 12, 2018

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

sciunto commented Sep 22, 2019

soupault left a 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

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

jni commented Dec 29, 2019

IssamLaradji commented Apr 21, 2020

rfezzani commented Apr 21, 2020

grlee77 commented Mar 27, 2022 • edited

rfezzani commented Mar 28, 2022

grlee77 commented Mar 27, 2022 •

edited