Decorators for helping with the multichannel->channel_axis transition

[grlee77]

This PR is related to #4294. It introduces two new decorators to help with the proposed transition from multichannel to channel_axis:

1.) deprecate_multichannel_kwarg is a decorated based on deprecate_kwarg, but additionally takes care of converting values from:
multichannel=False -> channel_axis=None
multichannel=True -> channel_axis=(-1,)

2.) channel_as_last_axis can be used to help automate shuffling of axes to/from the last position. This will not be needed for functdions capable of handling arbitrary channel positions, but is a quick way to adapt many existing functions that all assume channels correspond to the last axis.

For now I just applied these to the existing montage function as a concrete example. I want to get initial feedback before extending the approach further. Right now only a single channel axis is supported, but the channel_as_last_axis decorator could also be adapted to handle more general cases.

[pep8speaks]

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

• In the file skimage/util/tests/test_montage.py:

Line 66:12: E201 whitespace after '['
Line 66:15: E241 multiple spaces after ','
Line 67:12: E201 whitespace after '['
Line 67:15: E241 multiple spaces after ','
Line 68:12: E201 whitespace after '['
Line 68:15: E241 multiple spaces after ','
Line 70:12: E201 whitespace after '['
Line 70:15: E241 multiple spaces after ','
Line 71:12: E201 whitespace after '['
Line 71:15: E241 multiple spaces after ','
Line 74:12: E201 whitespace after '['
Line 74:15: E241 multiple spaces after ','
Line 75:12: E201 whitespace after '['
Line 75:15: E241 multiple spaces after ','
Line 76:12: E201 whitespace after '['
Line 76:15: E241 multiple spaces after ','
Line 77:12: E201 whitespace after '['
Line 77:15: E241 multiple spaces after ','
Line 78:12: E201 whitespace after '['
Line 78:15: E241 multiple spaces after ','
Line 79:12: E201 whitespace after '['
Line 79:15: E241 multiple spaces after ','
Line 80:12: E201 whitespace after '['
Line 80:15: E241 multiple spaces after ','
Line 81:12: E201 whitespace after '['
Line 81:15: E241 multiple spaces after ','
Line 125:11: E201 whitespace after '['
Line 125:16: E241 multiple spaces after ','
Line 125:22: E241 multiple spaces after ','
Line 125:28: E241 multiple spaces after ','

Comment last updated at 2021-03-05 13:21:09 UTC

[emmanuelle]

not sure what channel_pos is used for here

[grlee77]

It should not be there. I originally started modifying this test class, but later decided to leave this one using multichannel and add a new test_montage_simple_rgb_channel_axes test case instead to test the channel_axis argument.

[emmanuelle]

Thanks @grlee77 the approach looks sound to me. So deprecate_multichannel_kwarg would be used just during the transition to 1.0 but the channel_as_last_axis decorator could stay for a longer time? With the latter decorator and the use of np.moveaxis, do I understand correctly that for an array with channels corresponding to the leading axis, channel_axis=0 + this decorator would lead to memory-contiguous arrays being used in functions such as denoise_tv_bregman etc. which do

channel_in = np.ascontiguousarray(image[..., c:c+1])

?

[grlee77]

So deprecate_multichannel_kwarg would be used just during the transition to 1.0 but the channel_as_last_axis decorator could stay for a longer time?

Yes, this way we don't have to make solutions for every functions individually to support alternative channel positions. If there are cases where it makes sense to handle the channels in a different manner, then those functions can implement that internally and stop using the channel_as_last_axis decorator.

do I understand correctly that for an array with channels corresponding to the leading axis, channel_axis=0 + this decorator would lead to memory-contiguous arrays being used in functions such as denoise_tv_bregman etc. which do

Yes, If the input array is C-contiguous with channels as the first axis then after np.moveaxis to the last position the overall array is no longer C-contiguous, but the values for a specific channel will still be C-contiguous. Minimal example

import numpy as np

a = np.ones((3, 256, 256))
a_reordered = np.moveaxis(a, 0, -1)
# overall array is now no longer C-contiguous
print(a_reordered.flags)
# but invidual channels are C-contiguous
print(a_reordered[..., 0].flags)

Also a_reordered.flags.owndata is False indicating that a_reordered is a view of a, not a copy.

[emmanuelle]

Thanks @grlee77 the PR is ready as far as I'm concerned. @scikit-image/core we need another review please :-)

[stefanv]

Looks good to me; I guess we'll know more once we start using this in practice.

[grlee77]

I added a couple of additional functions to illustrate additional arguments for the channel_as_last_axis decorator

match_histograms : this one swaps channels for the first two positional arguments
apply_parallel: this one swaps channels for only the second positional argument

This PR now covers all multichannel use in skimage.util and skimage.exposure. To keep the number of changes relatively small, I would like to address other modules in subsequent PRs.

[jni]

@grlee77 I've left a minor suggestion. It'd be great if the docstrings were PEP257 compliant (first line must be a one-liner), but it's in private classes so 🤷 I really don't mind. =)

[grlee77]

Thanks @jni, I made the requested modifications.

[jni]

Thank you @grlee77! The failed checks appeared to be due to a stalled VM.