Add a channel_axis argument to functions in the skimage.color module #5462

grlee77 · 2021-07-07T21:01:51Z

Description

This PR adds a new channel_axis argument to color conversion functions as discussed a bit in #5439 and corresponding recent API-related meetings.

For most functions this is pretty simple, often just requiring adding the channel_axis keyword and the pre-existing channel_as_last_axis decorator. Most of the work was in modifying the test cases to test each function with additional channel axis choices.

One specific case that is a bit tricky is how to handle the adapt_rgb decorator so that it supports channels along any axis instead of always assuming the last axis. I implemented something for that in commit 8063049 that involves inspection of keyword arguments of the decorated function, but I think this is possibly too magical and is perhaps not worth it. It doesn't help that adapt_rgb is also one place where we apparently still try to guess if an image is RGB/RGBA based on its shape. The only function in the library that uses adapt_rgb is skimage.exposure.equalize_adapthist. If we don't want this change I can just revert that commit and/or leave it to a potential future follow-up PR.

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.

TST: test rgb2hsv and hsv2rgb with variable channel_axis

add channel_axis to rgb2rgbcie, rgbcie2rgb add channel_axis to gray2rgb, gray2rgba, rgb2gray, rgba2gray

add channel_axis to rgb2lab, lab2rgb, xyz2lab, lab2xyz

…value

support channel_axis for hsv_value and each_channel decorators

add channel_axis support to convert_colorspace

pep8speaks · 2021-07-07T21:01:56Z

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

In the file skimage/color/tests/test_colorconv.py:

Line 617:57: E225 missing whitespace around operator

Comment last updated at 2021-08-09 20:26:13 UTC

skimage/color/colorlabel.py

mkcor · 2021-07-17T14:21:56Z

One specific case that is a bit tricky is how to handle the adapt_rgb decorator so that it supports channels along any axis instead of always assuming the last axis.

I would be very tempted to drop skimage/color/adapt_rgb.py altogether.

It doesn't help that adapt_rgb is also one place where we apparently still try to guess if an image is RGB/RGBA based on its shape.

Exactly. It's not robust at all and it doesn't generalize to nD. I'm a little scared when I read: "A very simple check of the array shape is used for detecting RGB images, so adapt_rgb is not recommended for functions that support 3D volumes or color images in non-RGB spaces." (note at the bottom of https://scikit-image.org/docs/stable/auto_examples/color_exposure/plot_adapt_rgb.html where we document/encourage the use of the adapt_rgb decorator) 😱

The only function in the library that uses adapt_rgb is skimage.exposure.equalize_adapthist.

I searched all GitHub and found that this project by @lauraluebbert uses skimage.exposure.equalize_adapthist. I'll double check when Binder starts, but it looks like the function is used on single-channel inputs (even though the analyzed dataset consists of colour images).

grlee77 · 2021-07-17T16:24:18Z

Yeah I think it is best if I just rebase without commit 3d82e2c or 8063049 (I think all adaptrgb-related changes were in those two commits). We can decide about potentially dropping the decorator altogether elsewhere.

This reverts commit 8063049.

…and hsv_value" This reverts commit 3d82e2c.

grlee77 · 2021-07-17T16:27:46Z

I decided to use git revert instead of git rebase -i so the commits are still in the history

mkcor

I'm only half-way through, but it's getting late; will finish during the week!

skimage/color/adapt_rgb.py

skimage/_shared/utils.py

skimage/color/colorconv.py

mkcor

I'm confused: I couldn't find channel_as_last_axis defined in skimage/_shared/utils.py--where is it?

mkcor · 2021-07-26T18:58:42Z

skimage/color/tests/test_colorconv.py

+                channel_axis=channel_axis
+            ),
+            img_rgb,
+        )


skimage/color/colorconv.py

Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>

skimage/color/colorconv.py

mkcor · 2021-07-27T18:05:12Z

I'm confused: I couldn't find channel_as_last_axis defined in skimage/_shared/utils.py--where is it?

False alarm, sorry, I don't know what happened to my sed/grep... 😕

the pre-existing channel_as_last_axis decorator

line 220 of expected file in current main 😅

rfezzani

Thank you @grlee77, I left some suggestions, otherwise it LGTM 😉

skimage/_shared/utils.py

skimage/color/colorconv.py

rfezzani · 2021-08-04T09:35:55Z

skimage/color/colorconv.py

+        alpha = np.full(arr.shape, alpha, dtype=arr.dtype)
+    elif alpha.shape != arr.shape:
+        raise ValueError("alpha.shape must match image.shape")
+    rgba = np.stack((arr,) * 3 + (alpha,), axis=channel_axis)


❤️ Very elegant 😉

rfezzani · 2021-08-04T10:02:46Z

skimage/color/colorconv.py

    return rgba


-def gray2rgb(image, alpha=None):
+def gray2rgb(image, *, channel_axis=-1):


The alpha argument may had be removed in a different PR, but it may be more complicated. Please remove the corresponding item in TODO.txt 😉

Apparently I did already remove alpha and the TODO item as part of #5463 so perhaps we should merge that first to make this cleaner.

skimage/color/tests/test_colorconv.py

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>

grlee77 · 2021-08-05T15:27:03Z

Thanks @rfezzani, I implemented the requested changes in the last few commits.

rfezzani · 2021-08-06T07:25:55Z

@grlee77, if you prefer to merge #5463 prior to this one, there is a conflict in colorlabel.py there 😕

rfezzani · 2021-08-09T14:57:18Z

@grlee77, we will be ready for merge after conflict resolution 😉

rfezzani · 2021-08-09T23:11:59Z

🎉 Thank you again @grlee77

grlee77 added 13 commits July 2, 2021 12:58

TST: channel_axis tests for rgba2rgb

51b9dcb

Add channel_axis kwarg to rgb2hsv and hsv2rgb

4e73211

TST: test rgb2hsv and hsv2rgb with variable channel_axis

use existing channel_as_last_axis decorator for hsv2rgb and rgb2hsv

9996eb1

add channel_axis to rgb2xyz, xyz2rgb

954371e

add channel_axis to rgb2rgbcie, rgbcie2rgb add channel_axis to gray2rgb, gray2rgba, rgb2gray, rgba2gray

add channel_axis to rgb2luv, luv2rgb, xyz2luv, luv2xyz

9f76f7c

add channel_axis to rgb2lab, lab2rgb, xyz2lab, lab2xyz

add channel_axis to combine_stains, separate_stains, rgb2hed, hed2rgb

ec4ea97

add channel_axis support to remaining functions in colorconv.py

31ea1ed

support user-specified channel_axis for label2rgb

54ff440

support channel_axis for deltaE_*

37fd7ff

add channel_axis option to adapt_rgb decorator, each_channel and hsv_…

3d82e2c

…value

support use of channel_axis in functions wrapped by adapt_rgb

8063049

support channel_axis for hsv_value and each_channel decorators

DOC: update docstrings to describe channel_axis

a3d456e

add channel_axis support to convert_colorspace

BUG: fix label2rgb when an RGB image is provided and channel_axis != -1

bb21a4f

grlee77 added the ⏩ type: Enhancement Improve existing features label Jul 7, 2021

grlee77 changed the title ~~Color channel axis~~ Add a channel_axis argument to functions in the skimage.color module Jul 7, 2021

grlee77 added the 📜 type: API Involves API change(s) label Jul 8, 2021

grlee77 added 2 commits July 8, 2021 00:27

pep8 fixes

f0184fd

pep8 fixes

c72a767

grlee77 mentioned this pull request Jul 8, 2021

Meta-issue: pending API changes for 2.0 #5439

Closed

Merge remote-tracking branch 'upstream/main' into color-channel_axis

c3c9cc9

mkcor reviewed Jul 13, 2021

View reviewed changes

skimage/color/colorlabel.py Show resolved Hide resolved

grlee77 mentioned this pull request Jul 13, 2021

2021's calendar of community management #5169

Closed

grlee77 added 2 commits July 17, 2021 12:26

Revert "support use of channel_axis in functions wrapped by adapt_rgb"

501d929

This reverts commit 8063049.

Revert "add channel_axis option to adapt_rgb decorator, each_channel …

287150e

…and hsv_value" This reverts commit 3d82e2c.

update shape description in labl2rgb docstring

a40d108

mkcor reviewed Jul 18, 2021

View reviewed changes

skimage/color/adapt_rgb.py Show resolved Hide resolved

skimage/_shared/utils.py Outdated Show resolved Hide resolved

skimage/color/colorconv.py Outdated Show resolved Hide resolved

skimage/color/colorconv.py Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/main' into color-channel_axis

7f0f3a2

mkcor mentioned this pull request Jul 26, 2021

Colorconv tests cleaning #5364

Open

mkcor reviewed Jul 26, 2021

View reviewed changes

grlee77 and others added 2 commits July 26, 2021 15:44

Apply suggestions from code review

0155d1b

Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>

switch from ValueError to np.AxisError for invalid axis in rgba2rgb

444284d

mkcor reviewed Jul 26, 2021

View reviewed changes

skimage/color/colorconv.py Outdated Show resolved Hide resolved

raise TypeError on non-integer axis

e16510f

mkcor approved these changes Jul 27, 2021

View reviewed changes

mkcor added the action: mrg+1 label Jul 27, 2021

rfezzani reviewed Aug 4, 2021

View reviewed changes

grlee77 and others added 3 commits August 5, 2021 11:15

TST: test additional negative channel_axis values

5abb68e

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>

STYLE: rename _reshape_nd to reshape_nd

03a8650

move identity helper to skimage._shared.utils

8b504a7

rfezzani approved these changes Aug 6, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/main' into color-channel_axis

bfe1cd1

rfezzani merged commit 293fb93 into scikit-image:main Aug 9, 2021

grlee77 mentioned this pull request Sep 7, 2021

[FEA] Update API for consistency with upcoming scikit-image 0.19 rapidsai/cucim#98

Closed

grlee77 mentioned this pull request Dec 24, 2021

Add channel_axis as in scikit-image 0.19 rapidsai/cucim#188

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a channel_axis argument to functions in the skimage.color module #5462

Add a channel_axis argument to functions in the skimage.color module #5462

grlee77 commented Jul 7, 2021

pep8speaks commented Jul 7, 2021 •

edited

Loading

mkcor commented Jul 17, 2021

grlee77 commented Jul 17, 2021

grlee77 commented Jul 17, 2021

mkcor left a comment

mkcor left a comment

mkcor Jul 26, 2021

mkcor commented Jul 27, 2021

rfezzani left a comment

rfezzani Aug 4, 2021

rfezzani Aug 4, 2021

grlee77 Aug 5, 2021

grlee77 commented Aug 5, 2021

rfezzani commented Aug 6, 2021

rfezzani commented Aug 9, 2021

rfezzani commented Aug 9, 2021

Add a channel_axis argument to functions in the skimage.color module #5462

Add a channel_axis argument to functions in the skimage.color module #5462

Conversation

grlee77 commented Jul 7, 2021

Description

Checklist

For reviewers

pep8speaks commented Jul 7, 2021 • edited Loading

Comment last updated at 2021-08-09 20:26:13 UTC

mkcor commented Jul 17, 2021

grlee77 commented Jul 17, 2021

grlee77 commented Jul 17, 2021

mkcor left a comment

Choose a reason for hiding this comment

mkcor left a comment

Choose a reason for hiding this comment

mkcor Jul 26, 2021

Choose a reason for hiding this comment

mkcor commented Jul 27, 2021

rfezzani left a comment

Choose a reason for hiding this comment

rfezzani Aug 4, 2021

Choose a reason for hiding this comment

rfezzani Aug 4, 2021

Choose a reason for hiding this comment

grlee77 Aug 5, 2021

Choose a reason for hiding this comment

grlee77 commented Aug 5, 2021

rfezzani commented Aug 6, 2021

rfezzani commented Aug 9, 2021

rfezzani commented Aug 9, 2021

pep8speaks commented Jul 7, 2021 •

edited

Loading