Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API inconsistency between remove_small_holes and remove_small_objects #4003

Open
sciunto opened this issue Jul 10, 2019 · 3 comments
Open

API inconsistency between remove_small_holes and remove_small_objects #4003

sciunto opened this issue Jul 10, 2019 · 3 comments
Labels
🥾 Path to skimage2 A step towards the new "API 2.0" 📜 type: API Involves API change(s)

Comments

@sciunto
Copy link
Member

sciunto commented Jul 10, 2019

Description

In 0.16, in remove_small_holes, min_size has been replaced definitely by area_threshold. See here.

But for remove_small_objects, it is still min_size. Why that? The first one call the other internally...

@jni
Copy link
Member

jni commented Jul 26, 2019

@sciunto personally I disagreed with the change to area_threshold, but didn't catch that PR in time. Either way, it's likely that remove_small_objects was simply overlooked. Thanks for flagging this. We should have a long discussion about the best name for these parameters.

@jni jni added this to the 1.0 milestone Jul 26, 2019
@jni jni changed the title API inconsistancy between remove_small_holes and remove_small_objects? API inconsistency between remove_small_holes and remove_small_objects Jul 26, 2019
@grlee77 grlee77 added the 📜 type: API Involves API change(s) label Jan 25, 2021
@grlee77 grlee77 added this to To Do in skimage2 API Jan 27, 2021
@mkcor
Copy link
Member

mkcor commented Feb 1, 2021

I think that min_size is/was fine as long as documentation is well written.

My second favourite option would be #4761 (comment) by @jni because it reads so well.

@lagru
Copy link
Member

lagru commented Feb 28, 2024

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.

I'll make a PR to get this going again. Please, feel very welcome to suggest an alternate concrete action to address this issue. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🥾 Path to skimage2 A step towards the new "API 2.0" 📜 type: API Involves API change(s)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants