Complete deprecations targeting release 0.20 or 1.0 #6583

lagru · 2022-10-16T20:02:52Z

Description

Complete deprecations that target 0.20. This PR addresses deprecations which were created for 0.19 with target 1.0. At that time a release of 0.20 wasn't planned. See also https://discuss.scientific-python.org/t/559.

Probably easiest to check the changes commit by commit (I tried to keep related changes together / rebased and squashed related changes).

Address whole multichannel deprecation & remove deprecate_multichannel_kwarg decorator class
*_iter -> *_num_iter in unsupervised_wiener
deprecate height and width in morphology.rectangle
TODO.txt -> only address deprecations to keep this focused

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
Descriptive commit messages (see below)

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.
There is a bot to help automate backporting a PR to an older branch. For
example, to backport to v0.19.x after merging, add the following in a PR
comment: @meeseeksdev backport to v0.19.x
To run benchmarks on a PR, add the run-benchmark label. To rerun, the label
can be removed and then added again. The benchmark output can be checked in
the "Actions" tab.

in favor of footprint. Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

stefanv · 2022-10-17T19:13:10Z

Thanks for working through these!

lagru · 2022-10-18T13:48:45Z

skimage/_shared/filters.py

 def gaussian(image, sigma=1, output=None, mode='nearest', cval=0,
-             multichannel=None, preserve_range=False, truncate=4.0, *,
+             preserve_range=False, truncate=4.0, *,
             channel_axis=None):


I'd like to point out

scikit-image/skimage/_shared/filters.py

Lines 107 to 112 in 8ae85b2

if image.ndim == 3 and image.shape[-1] == 3 and channel_axis is None:

msg = ("Images with dimensions (M, N, 3) are interpreted as 2D+RGB "

"by default. Use `multichannel=False` to interpret as "

"3D image with last dimension of length 3.")

warn(RuntimeWarning(msg))

channel_axis = -1

I am pretty sure that the logic makes it impossible to indicate to the function to treat an image of shape (M, N, 3) as a grayscale image as channel_axis=None is replaced with channel_axis=-1. The warning text implies, that in the absence of an explicit multichannel, the image is treated as RGB in some cases. However, with the switch to channel_axis we can no longer replicate this behavior. channel_axis=None means "no RGB" as opposed to multichannel=None which means "not set".

Pretty sure this was broken since the deprecate_multichannel_kwarg was applied as well. A that one translates multichannel=False into channel_axis=None...

My proposal is to remove this (broken) implicit RGB assumption if (M, N, 3).

Oops! The warning message is problematic too; it should have been updated ever since multichannel became deprecated, so the user isn't invited to set a deprecated kwarg...

We need another None-proxy, e.g.:

class NoChannelAxis: pass def gaussian(..., channel_axis=NoChannelAxis): if channel_axis is NoChannelAxis: ... # User can do: gaussian(..., channel_axis=None)

See a5421d5. Patching the __repr__ with a metaclass was actually a lot of fun, although it probably shouldn't be. 😅

I think the patch addresses the following cases:

User neither passed multichannel nor channel_axis and relied on automatic detection of color channel for (M, N, 3). Since v0.19 we silently broke his workflow. This patch recovers the original behavior for one release and notifies the user.

User continued using multichannel=False. Since v0.19 that was replaced with channel_axis=None. In case the shape was (M, N, 3) we broke his code otherwise he was fine. This user is notified as well because multichannel is no longer available.

User read the misleading RuntimeWarning, and nevertheless started using channel_axis=None for a grayscale image. The warning continued. In case the shape was (M, N, 3) we broke his code otherwise he was fine. This patch recovers the original behavior but does not notify him.

All other cases using grayscale images shouldn't have been impacted.

@stefanv @lagru What is the status of this? It would be good to get this merged soon.

Are you asking about the status of this specific comment thread? It's been addressed in a5421d5.

Concerning the status of this PR, I think I addressed everything I found at the time. The only remaining suggestion was to add .. versionchanged:: directives everywhere. But that should not hold up this PR in my opinion and can be done here or in another PR if someone finds the time.

Thank you for patching up this behavior @lagru, it looks great.

skimage/segmentation/_clear_border.py

skimage/morphology/misc.py

in favor of "out". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

in favor of "max_num_iter". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

in favor of "num_iter". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

in favor of "max_num_iter". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

in favor of "label_image". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Though the default removed_version of deprecate_multichannel_kwarg points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

This decorator is no longer necessary since the deprecation of multichannel is completed with 0.20 (previously 1.0). Make test's independent of temporary decorations of library functions such as hog and pyramid_gaussian. Their deprecation decorator was removed.

and remove unused imports.

For filters.inverse(), the removed_version targets 0.21 and not 0.20. So move.

I think removed_version="1.2" is actually supposed to be 1.0 (or 0.20).

These deprecations are already part of 0.19. So it should be fine to remove them in 0.20.

skimage/measure/_regionprops_utils.py

TODO.txt

Missed this earlier.

lagru · 2022-10-19T11:06:16Z

@scikit-image/core this is ready for review. Please have a look. Reviewing commit by commit might be useful as I tried to keep related changes together...

The fix for the failing test is discussed in #6583 (comment).

mkcor · 2022-10-19T14:37:56Z

skimage/filters/_median.py

 from .rank import generic


-@deprecate_kwarg(kwarg_mapping={'selem': 'footprint'}, removed_version="1.0",
-                 deprecated_version="0.19")


Before the 'Returns' section, shouldn't there be something like:

.. versionchanged:: 0.19 Parameter ``selem`` has been replaced by ``footprint``.

? I think @jni would know...

I'm pretty sure that we don't use the .. versionchanged:: directive consistently (yet). But I could go through and update each docstring if we deem it worth it to keep the notice around.

This reverts commit 56f0431. The deprecation of `compute_hessian_eigenvalues` was introduced after all 0.19.x releases (see git tag --contains d2689f5)! So the target should be 0.21.

Automatic detection of the color channel based on the old deprecated `multichannel=None` was broken in version 0.19. This commit recovers the old behavior for 0.20 using a proxy object `ChannelAxisNotSet`. The behavior is still deprecated though and should be removed completely in 0.21. Note that the warning is only raised if the automatic behavior deviates from the behavior that will be the new default in 0.21, which is `channel_axis=None`. This way we only notify users that were affected by the broken behavior in 0.19 and now deprecated behavior in 0.20.

in `regionprops` (since 0.16) and `active_contour` (since 0.18).

Instead of raising a ValueError, since f8dfea6 [1] this is addressed by returning `False`. [1] scikit-image#6453

# Conflicts: # skimage/restoration/_denoise.py # skimage/segmentation/tests/test_random_walker.py

These were removed in d951774 and b53b46a.

grlee77

Thank you for completing all of these @lagru. I am especially happy to see the deprecation cycle completed for both multichannel and selem!

…sian

grlee77 · 2022-11-18T18:21:05Z

The one CI failure was due to an uncaught FutureWarning message. I pushed a commit that should fix it.

…cations

stefanv

Thanks for taking care of this @lagru!

Just one request to clarify a docstring, otherwise LGTM.

stefanv · 2022-11-22T20:58:38Z

skimage/_shared/filters.py

+        return f"<{cls.__name__}>"
+
+
+class ChannelAxisNotSet(metaclass=_PatchClassRepr):


Is the metaclass necessary?

I'm sure there are other ways to control how this signal object is rendered in the signature (I would post the link but strangely the pipeline did not build the docs?). I initially included the __repr__ in this class itself which requires creating an instance of it in the same scope to check for identity in the function body. Compared to that, I actually found the metaclass-based approach clearer and easier to document. The class itself clearly is the signal and there is no confusion about a second symbol that also looks like a signal; it's clearly a metaclass only responsible for modifying the representation.

It's not something I feel strongly about, though. The verbose answer is mainly because I'm rationalizing my gut feeling about which approach "felt better" in retrospective. 😄

skimage/_shared/filters.py

Co-authored-by: Stefan van der Walt <sjvdwalt@gmail.com>

grlee77 · 2022-11-29T17:55:43Z

I think this should be good to go now. Some of these deprecations (e.g. multichannel and selem) will be great to have completed. However, note that they were first introduced in 0.19, so removing them now would be quicker than our usual two-release deprecation cycle. Given that it has been a full year since 0.19.0 was released, I am in favor of it, though. Are there any objections, or should we merge this now?

stefanv · 2022-11-29T18:46:04Z

I'm in favor of merging.

mkcor · 2022-11-30T09:39:41Z

I'm also in favour of merging this PR.

grlee77 · 2022-11-30T17:00:48Z

Merged, thanks @lagru!

The `multichannel` field in `skimage.util.montage` has been deprecated in version 0.20. Update function call to work with the new API. From the relase notes for 0.20: Remove deprecated parameter multichannel; use channel_axis instead. [...] In skimage.util, this affects montage and apply_parallel ([#6583](scikit-image/scikit-image#6583)).

lagru added 2 commits October 16, 2022 21:10

Complete deprecation of selem parameter

7430c41

in favor of footprint. Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of selem module

d951774

Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

lagru added 🔧 type: Maintenance Refactoring and maintenance of internals 🔽 Deprecation Involves deprecation labels Oct 16, 2022

lagru added this to the 0.20 milestone Oct 16, 2022

lagru commented Oct 18, 2022

View reviewed changes

skimage/segmentation/_clear_border.py Show resolved Hide resolved

skimage/morphology/misc.py Show resolved Hide resolved

lagru added 14 commits October 19, 2022 11:32

Complete deprecation of in_place parameter

64e0023

in favor of "out". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of max_iterations parameter

198566f

in favor of "max_num_iter". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of max_iter parameter

6db637d

in favor of "max_num_iter". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of iterations parameter

50cdfb0

in favor of "num_iter". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of n_iter_max parameter

4cb8866

in favor of "max_num_iter". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of compute_hessian_eigenvalues

56f0431

Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of input parameter

f77d0da

in favor of "label_image". Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of greyco* functions

171d34a

Though removed_version points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of multichannel kwarg

9b2c8f0

Though the default removed_version of deprecate_multichannel_kwarg points at 1.0, the deprecation is finished in 0.20 which was not planned at the time of 0.19's release.

Complete deprecation of height, width in rectangle

c25941e

and remove unused imports.

Remove completed items in TODO.txt

1beab4a

For filters.inverse(), the removed_version targets 0.21 and not 0.20. So move.

Complete deprecation of neighbourhood parameter

6a24f27

I think removed_version="1.2" is actually supposed to be 1.0 (or 0.20).

Remove deprecated grey and greyreconstruct modules

b53b46a

These deprecations are already part of 0.19. So it should be fine to remove them in 0.20.

lagru force-pushed the complete-0.20-deprecations branch from 7462c13 to b53b46a Compare October 19, 2022 09:32

lagru commented Oct 19, 2022

View reviewed changes

skimage/measure/_regionprops_utils.py Show resolved Hide resolved

lagru commented Oct 19, 2022

View reviewed changes

TODO.txt Outdated Show resolved Hide resolved

Remove warning about deprecated *_iter

38c23c4

Missed this earlier.

lagru marked this pull request as ready for review October 19, 2022 11:06

mkcor reviewed Oct 19, 2022

View reviewed changes

mkcor mentioned this pull request Oct 19, 2022

Rewrite docstring for denoise_tv_chambolle #6544

Merged

lagru added 7 commits October 25, 2022 15:14

Revert "Complete deprecation of compute_hessian_eigenvalues"

998ee00

This reverts commit 56f0431. The deprecation of `compute_hessian_eigenvalues` was introduced after all 0.19.x releases (see git tag --contains d2689f5)! So the target should be 0.21.

Complete deprecation of coordinates parameter

d752adf

in `regionprops` (since 0.16) and `active_contour` (since 0.18).

Remove outdated TODO for CircleModel.estimate

206e599

Instead of raising a ValueError, since f8dfea6 [1] this is addressed by returning `False`. [1] scikit-image#6453

Merge branch 'main' into complete-0.20-deprecations

bc476b7

# Conflicts: # skimage/restoration/_denoise.py # skimage/segmentation/tests/test_random_walker.py

Remove removed files from meson.build

9b70ed6

These were removed in d951774 and b53b46a.

Remove obsolete tests for coordinates parameter

e7d8c75

grlee77 approved these changes Nov 18, 2022

View reviewed changes

Catch expected warning for deprecated color channel inference in gaus…

15638d4

…sian

grlee77 added 2 commits November 18, 2022 13:58

fix typo in prior commit

cc32ae2

Merge remote-tracking branch 'upstream/main' into complete-0.20-depre…

dc843fe

…cations

jarrodmillman approved these changes Nov 22, 2022

View reviewed changes

stefanv approved these changes Nov 22, 2022

View reviewed changes

lagru and others added 2 commits November 23, 2022 15:47

Update skimage/_shared/filters.py

246bc46

Co-authored-by: Stefan van der Walt <sjvdwalt@gmail.com>

Explain proxy value in more clearly

529c74a

grlee77 mentioned this pull request Nov 29, 2022

Implement additional deprecations carried out for scikit-image 0.20 rapidsai/cucim#451

Merged

grlee77 merged commit a91f4f8 into scikit-image:main Nov 30, 2022

lagru deleted the complete-0.20-deprecations branch December 1, 2022 09:51

lagru mentioned this pull request Dec 7, 2022

Prepare release notes for v0.20.0 #6556

Merged

4 tasks

wyli mentioned this pull request Mar 1, 2023

TypeError: regionprops() got an unexpected keyword argument 'coordinates' Project-MONAI/MONAI#6084

Closed

joshuacwnewton mentioned this pull request Mar 2, 2023

"ImportError: cannot import name greycomatrix from skimage.feature" in sct_analyze_texture spinalcordtoolbox/spinalcordtoolbox#4056

Closed

This was referenced Mar 30, 2023

Update montage2d to scikit-image 0.20 orientnab/MedMNIST#1

Merged

Update montage2d to scikit-image 0.20 MedMNIST/MedMNIST#29

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Complete deprecations targeting release 0.20 or 1.0 #6583

Complete deprecations targeting release 0.20 or 1.0 #6583

lagru commented Oct 16, 2022 •

edited

stefanv commented Oct 17, 2022

lagru Oct 18, 2022

mkcor Oct 19, 2022

stefanv Oct 25, 2022

lagru Oct 26, 2022

jarrodmillman Nov 17, 2022

lagru Nov 17, 2022 •

edited

grlee77 Nov 18, 2022

lagru commented Oct 19, 2022

mkcor Oct 19, 2022

lagru Oct 19, 2022 •

edited

grlee77 left a comment

grlee77 commented Nov 18, 2022 •

edited

stefanv left a comment

stefanv Nov 22, 2022

lagru Nov 23, 2022 •

edited

grlee77 commented Nov 29, 2022

stefanv commented Nov 29, 2022

mkcor commented Nov 30, 2022

grlee77 commented Nov 30, 2022

	if image.ndim == 3 and image.shape[-1] == 3 and channel_axis is None:
	msg = ("Images with dimensions (M, N, 3) are interpreted as 2D+RGB "
	"by default. Use `multichannel=False` to interpret as "
	"3D image with last dimension of length 3.")
	warn(RuntimeWarning(msg))
	channel_axis = -1

		return f"<{cls.__name__}>"


		class ChannelAxisNotSet(metaclass=_PatchClassRepr):

Complete deprecations targeting release 0.20 or 1.0 #6583

Complete deprecations targeting release 0.20 or 1.0 #6583

Conversation

lagru commented Oct 16, 2022 • edited

Description

Checklist

For reviewers

stefanv commented Oct 17, 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

lagru Nov 17, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lagru commented Oct 19, 2022

Choose a reason for hiding this comment

lagru Oct 19, 2022 • edited

Choose a reason for hiding this comment

grlee77 left a comment

Choose a reason for hiding this comment

grlee77 commented Nov 18, 2022 • edited

stefanv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lagru Nov 23, 2022 • edited

Choose a reason for hiding this comment

grlee77 commented Nov 29, 2022

stefanv commented Nov 29, 2022

mkcor commented Nov 30, 2022

grlee77 commented Nov 30, 2022

lagru commented Oct 16, 2022 •

edited

lagru Nov 17, 2022 •

edited

lagru Oct 19, 2022 •

edited

grlee77 commented Nov 18, 2022 •

edited

lagru Nov 23, 2022 •

edited