Use `max_size` in `remove_small_(holes|objects)` by lagru · Pull Request #7739 · scikit-image/scikit-image

lagru · 2025-03-05T11:13:13Z

Description

Closes #4003.

In 0.16, in skimage.morphology.remove_small_holes, min_size has been replaced definitely by area_threshold. But for skimage.morphology.remove_small_objects, it is still min_size. This attempts to solve that inconsistency by using a more sensible max_size parameter for both. At the same time, this threshold is now inclusive, meaning objects equal to it will be removed as well.

Having dug through the past issues and discussions on this, it seems to me that each discussed option will be a trade-off between consistency and matching the context of each function.

Since remove_small_objects and remove_small_holes are so similar, I definitely give consistency more weight. Ignoring past deprecations, min_size seems to be the least controversial option. The main argument against it was that we'd have to revert a previous deprecation.

That's not ideal, but addressing previous mistakes is precisely what skimage2 is about. So, I propose to deprecate area_threshold in remove_small_holes back to min_size and only finalize the deprecation in 2.0.0.

I think we can keep using area_threshold in area_opening and area_closing, since area is a prominent term and I anticipate less user confusion.

— #4003 (comment)

This comment got two thumbs up so I'm going with that suggestion.

Checklist

A descriptive but concise pull request title
Docstrings for all functions
Unit tests
A gallery example in ./doc/examples for new features
Contribution guide is followed

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

In `skimage.morphology.remove_small_objects`, deprecate the `min_size` parameter
in favor of the new `max_size` parameter to make API and behavior clearer. This new
threshold removes objects smaller than **or equal to** its value, while the
previous parameter only removed smaller ones.

In `skimage.morphology.remove_small_holes`, deprecate the `area_threshold` parameter
in favor of the new `max_size` parameter to make API and behavior clearer. This new
threshold removes holes smaller than **or equal to** its value, while the
previous parameter only removed smaller ones.

In 0.16, in remove_small_holes, min_size has been replaced definitely by area_threshold. But for remove_small_objects, it is still min_size. This attempts to solve that inconsistency. Having dug through the past issues and discussions on this, it seems to me that each discussed option will be a trade-off between consistency and matching the context of each function. Since remove_small_objects and remove_small_holes are so similar, I definitely give consistency more weight. Ignoring past deprecations, min_size seems to be the least controversial option. The main argument against it was that we'd have to revert a previous deprecation. That's not ideal, but addressing previous mistakes is precisely what skimage2 is about. So, I propose to deprecate area_threshold in remove_small_holes back to min_size and only finalize the deprecation in 2.0.0. I think we can keep using area_threshold in area_opening and area_closing, since area is a prominent term and I anticipate less user confusion.

skimage/morphology/misc.py

> From remove_small_holes(a, min_size=...), I don't think a user can > figure out what min_size means, whereas > remove_small_holes(a, max_size=...) reads pretty clearly as > "remove small holes, but don't go beyond max_size". – Stéfan

Co-authored-by: Stefan van der Walt <github@mentat.za.net>

mkcor

I'll submit this for now, seeing that I'm supposed to refresh the page of changed files.

skimage/morphology/misc.py

as a side effect, the existing argument "connectivity" can be made keyword-only as well.

mkcor

Thanks, @lagru!

I noticed there was a strange sentence under Notes for function remove_small_holes ("It is suggested that labeling is completed after using this function.") but this is probably being the scope of this PR. I'm happy to create an issue for this, so it doesn't fall in the cracks.

skimage/morphology/misc.py

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

Co-authored-by: Stefan van der Walt <github@mentat.za.net>

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

skimage/morphology/misc.py

Co-authored-by: Stefan van der Walt <stefanv@berkeley.edu>

mkcor

Almost there!

doc/examples/features_detection/plot_remove_objects.py

skimage/morphology/misc.py

skimage/morphology/tests/test_misc.py

skimage/morphology/misc.py

skimage/morphology/tests/test_misc.py

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

lagru · 2025-03-16T13:36:30Z

Thanks for catching the places were I forgot to decrement the max_size by one. I've found two more. Should be ready to go now. 🤞

mkcor

I found three leftovers!

skimage/morphology/tests/test_misc.py

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

mkcor

🎉

…s deprecation (#8003) Fixes #8001 The renaming of `min_size`/`area_threshold` to `max_size` in #7739 was actually accompanied by a change in behaviour for the same value, due to using `>` in the old nomenclature and `>=` in the new one. Unfortunately our deprecation machinery was until now not smart enough to know whether a renamed parameter was provided under its old name or its new name. To address this, this PR: - Updates the deprecation machinery to detect when a parameter was given with the old name/position, and - Uses this updated machinery to modify `min_size`/`area_threshold` by 1 to ensure that the new behaviour matches the old one. Co-authored-by: Jeremy Muhlich <jmuhlich@bitflood.org>

lagru added 🔽 Deprecation Involves deprecation 📜 type: API Involves API change(s) 🥾 Path to skimage2 A step towards the new "API 2.0" labels Mar 5, 2025

lagru added this to skimage2 Mar 5, 2025

github-project-automation bot moved this to To Do in skimage2 Mar 5, 2025

lagru moved this from To Do to To Review in skimage2 Mar 5, 2025

lagru requested review from mkcor and stefanv March 5, 2025 11:14

lagru added this to the 0.26 milestone Mar 5, 2025

Use min_size in tests instead of area_threshold

5640ed9

stefanv reviewed Mar 7, 2025

View reviewed changes

skimage/morphology/misc.py Outdated Show resolved Hide resolved

skimage/morphology/misc.py Outdated Show resolved Hide resolved

skimage/morphology/misc.py Outdated Show resolved Hide resolved

lagru changed the title ~~Deprecate area_threshold in remove_small_holes~~ Use max_size in remove_small_(holes|objects) Mar 7, 2025

Use "contiguous" in docstring

8673f21

Co-authored-by: Stefan van der Walt <github@mentat.za.net>

lagru requested a review from stefanv March 8, 2025 17:03

mkcor reviewed Mar 11, 2025

View reviewed changes

skimage/morphology/misc.py Outdated Show resolved Hide resolved

skimage/morphology/misc.py Outdated Show resolved Hide resolved

Make new max_size parameter keyword-only

332e86c

as a side effect, the existing argument "connectivity" can be made keyword-only as well.

lagru force-pushed the deprecate_area_threshold branch from 2997be7 to 332e86c Compare March 11, 2025 15:49

mkcor reviewed Mar 11, 2025

View reviewed changes

skimage/morphology/misc.py Outdated Show resolved Hide resolved

skimage/morphology/misc.py Outdated Show resolved Hide resolved

skimage/morphology/misc.py Outdated Show resolved Hide resolved

skimage/morphology/misc.py Outdated Show resolved Hide resolved

Tweak docstring and deprecation warnings

7218dbe

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

lagru requested a review from mkcor March 11, 2025 22:00

lagru and others added 3 commits March 12, 2025 14:20

Tweak parameter description further

c3da6ac

Co-authored-by: Stefan van der Walt <github@mentat.za.net>

Merge branch 'main' into deprecate_area_threshold

39438ab

Tweak docstring

a473309

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

stefanv reviewed Mar 12, 2025

View reviewed changes

skimage/morphology/misc.py Outdated Show resolved Hide resolved

stefanv approved these changes Mar 12, 2025

View reviewed changes

skimage/morphology/misc.py Outdated Show resolved Hide resolved

Tweak docstrings

50366c7

Co-authored-by: Stefan van der Walt <stefanv@berkeley.edu>

mkcor reviewed Mar 13, 2025

View reviewed changes

lagru commented Mar 16, 2025

View reviewed changes

skimage/morphology/tests/test_misc.py Outdated Show resolved Hide resolved

lagru commented Mar 16, 2025

View reviewed changes

skimage/morphology/tests/test_misc.py Outdated Show resolved Hide resolved

lagru commented Mar 16, 2025

View reviewed changes

skimage/morphology/tests/test_misc.py Outdated Show resolved Hide resolved

Add small corrections

5a59d9a

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

lagru requested a review from mkcor March 16, 2025 13:36

mkcor reviewed Mar 17, 2025

View reviewed changes

skimage/morphology/tests/test_misc.py Outdated Show resolved Hide resolved

skimage/morphology/tests/test_misc.py Outdated Show resolved Hide resolved

skimage/morphology/tests/test_misc.py Outdated Show resolved Hide resolved

Decrement more max_size values

41fd0d9

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

lagru force-pushed the deprecate_area_threshold branch from 1f616da to 41fd0d9 Compare March 17, 2025 14:38

mkcor approved these changes Mar 17, 2025

View reviewed changes

mkcor merged commit 17cb184 into scikit-image:main Mar 17, 2025
23 checks passed

github-project-automation bot moved this from To Review to Done in skimage2 Mar 17, 2025

lagru deleted the deprecate_area_threshold branch March 17, 2025 15:06

mkcor mentioned this pull request Mar 26, 2025

Use workers instead of alternate parameter names #7302

Merged

jni mentioned this pull request Jan 14, 2026

Restore original behavior of min_size in remove_small_* during its deprecation #8003

Merged

Uh oh!

Conversation

lagru commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Release note

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lagru commented Mar 16, 2025

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lagru commented Mar 5, 2025 •

edited

Loading