Replace square, rectangle, cube with footprint_rectangular

[lagru]

Description

We have a rework of our footprint generating function on our skimage2 agenda. This WIP starts that work by combining "rectangular" footprint generators (square, rectangle, cube) into a single skimage.morphology.footprint_rectangular with ND support.

This is also an opportunity to change to decomposition="sequence" as the default as suggested in the agenda. Though, I'm not sure if we really want to do this. It's arguably a more unexpected form to represent the footprint for the sake of performance. Usually, we decide that trade-off in favor of simpler behavior.

I plan to do the same for "elliptical" footprint generators in a separate PR to keep reviewing reasonable.

I'm also playing around with a small wrapper class to help with the more complicated representation of decomposed footprints. I might move that into another PR depending on how this approach turns out.

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.

Add the new `skimage.morphology.footprint_rectangle`
supporting generation of rectangular or hyper-rectangular footprints
in one function. {label="New feature"}

Deprecate `skimage.morphology.square` in favor of the new function
`skimage.morphology.footprint_rectangle`. {label="API"}

Deprecate `skimage.morphology.rectangle` in favor of the new function
`skimage.morphology.footprint_rectangle`. {label="API"}

Deprecate `skimage.morphology.cube` in favor of the new function
`skimage.morphology.footprint_rectangle`. {label="API"}

[lagru]

I'm thinking how to best go about updating the test suite considering review experience and long-term maintenance.

The test's are somewhat strewn about over at least 2 files. So my idea is to consolidate them in a new Test_footprint_rectangle class but keep the old ones around during the deprecation. That way we make sure that the replacement really fully implements the old API. When we finish the deprecation, we just remove them and are left with a clearer grouping of tests.

[hmaarrfk]

Have "nd extensions" been using in your "actual work"? I have mostly stayed on "2D" + broadcasting.

I find "square rectangle and cube" relatable, and "footprint_rectangle" just too abstract....

[stefanv]

We are planning on renaming all structuring elements to footprint_, so that part at least will be more obvious.

[lagru]

My personal gut feeling is that rectangle isn't too abstract for our users to understand that it also entails squares. I could live with keeping footprint_cube as a 3d-specific alias and common use case of the general algorithm in footprint_rectangle around.

But thinking about it more, I'd actually find footprint_rectangular (and footprint_elliptical) to be perfect names. They aren't specific to a number of dimensions and I think communicate the core concept of the footprint class clearly.

Meta: I think the original aim of the discussion was to make our API clearer and reduce the maintenance aspect of it. While working on this, I discovered that the "rectangular" approach can actually be covered by one ND-enabled algorithm. I feel like that one generalized version is actually simpler to maintain long-term.

@hmaarrfk, what do you think?

[hmaarrfk]

@lagru all green with me.

[jarrodmillman]

pre-commit.ci autofix

[lagru]

Note this new warning which is raised when a sequence-decomposition is requested for footprints that have a dimension with an uneven length. This doesn't change previous behavior but makes it more visible to users what's going on.

[stefanv]

This LGTM. I think the name footprint_rectangle would be clearer, and aligns with previously used names cube, square, etc. This will also help us name the others footprint_diamond, footprint_sphere, etc.

[lagru]

Hmm, I'm rather fond of "rectangular" which works for any number of dimensions, e.g. rectangular cuboid, square, hyper-cube. footprint_rectangle makes it less clear that this function may also work for more than 2D.

Along the same line of thought, I'd suggest footprint_elliptical for ellipsis, sphere, ball, etc.

I think we only support 2D for diamond, octagon, etc. So going with a more precise name rather than a property makes sense here in my book.

What do you think?

[stefanv]

I think that's the problem: rectangular means rectangular something, and the name doesn't say what. footprint_hyperrect may have been most correct, and perhaps hyper is a way to distinguish from 2D footprints. Not sure what the correct form of "diamond" would be if we went with "rectangular". Also just extra letters and not the most direct form.

We could also consider footprint_rect_nd, footprint_diamond etc.

[mkcor]

The generalization makes sense to me. Nice work, @lagru!

[mkcor]

Suggested change

	The length of the footprint in each dimension. The length of the
	sequence determines the number of dimensions of the footprint.
	The size of the footprint in each dimension. The length of `shape`
	determines the number of dimensions of the footprint.

[mkcor]

No big deal, I just find it 'visually' confusing to read/see "The length ..." and then again, next sentence, "The length ..." when really they don't refer to the same thing at all.

[mkcor]

Suggested change

	footprints is returned. Applying this series of smaller footprints will
	give an identical result to a single, larger footprint, but often with
	footprints is returned. Applying this sequence of smaller footprints will
	give a result identical to applying a single, larger, equivalent footprint, but often with

[mkcor]

Suggested change

	If None, a single array is returned. For 'sequence', a tuple of smaller
	If None, a single array is returned. With 'sequence', a tuple of smaller

[mkcor]

Suggested change

	footprint : array or tuple[tuple[ndarray, int], ...]
	footprint : array or tuple[tuple[array, int], ...]

or ndarray or tuple[tuple[array, int], ...]?

[mkcor]

Suggested change

	A footprint consisting only of ones, i.e. every pixel belongs to the
	A footprint consisting only of ones, i.e., every pixel belongs to the

[mkcor]

Suggested change

	unique structuring elements to apply (see Examples for more detail).
	unique footprints to apply (see Examples for more detail).

Or the fact that we speak of 'structuring elements' and 'footprints' interchangeably should be introduced earlier/clearly.

[lagru]

Thanks @mkcor! Do you have a preference or thoughts with regard to footprint_rectangular and footprint_rectangle?

--

I think that's the problem: rectangular means rectangular something, and the name doesn't say what.

@stefanv, I can follow that argument. It may be somewhat alleviated by the fact, that footprint is included in the function name, which identifies the "something".

I still feel like rectangular is the more precise term, but I could live with using _rectangle. In that case, I'd advocate adding footprint_cuboid so that users looking for a 3D option can quickly find it. Then the two most common use cases should be covered.

[mkcor]

@lagru oh sorry, I'm seeing this conversation only now! I just checked and the second meaning of adjective 'rectangular' is: placed or having parts placed at right angles (so the 'cuboid' case would be covered, in the broad sense).

My personal preference would go to footprint_rect because it's 'safe' and 'inclusive' (may stand for rectangular, rectangle, hyperrect, rect_nd... knowing that our functions are typically nD by default).

[ivanov]

drive by peanut gallery comment (came here from the Scientific Python discord that solicited input here, because who doesn't love a good bike shed?):

I think that's the problem: rectangular means rectangular something, and the name doesn't say what.

In one interpretation, it does: rectangular footprint, where a user can assume you're using a noun_adjective convention to keep all of the nouns together for tab-completion usage and discovery convenience. This then raises the point of what the other footprints will be renamed to. Will it be footprint_sphere or footprint_spherical?

footprint_rect is a nice compromise, practically speaking. You could have a convention of using the 2d projection names for n-dimensional version, but if you stuck to that, would you want to change footprint_sphere to footprint_circle? An the annulus that looks like currently gets called disk - should you keep calling it that in an n-dimensional case, where it should perhaps be better described as a spherical shell?

I really don't have a dog in this race, or a say, but I would leave the "legacy" specialized names, or at least their footprint_ prefixed version to help those that have been using those all along, and cross reference the relevant generalization in their docstrings

Note: footprint_square is a special case of footprint_rect for 2 dimensions having the same size.

This way current users of those functions can continue to use them and think about their problems in the same way, only having to adjust to the footprint_ prefix. You could sprinkle in an optional n_dim or something like that where the hyper-shape makes sense. footprint_cube(3, n_dim=10) is pretty unambiguous, and a more pleasant API than the technically the same footprint_rectangular ((3, 3, 3, 3, 3, 3, 3, 3, 3, 3)), even if you shorten it to footprint_rectangular([3]*10), it's still iffy. I think it would be unergonomic to be forced to use footprint_elliptical to specify hyperspheres, even if that's how they'd be implemented internally.

[lagru]

leave the "legacy" specialized names, or at least their footprint_ prefixed version to help those that have been using those all along, and cross reference the relevant generalization in their docstrings

That's a good point I hadn't really considered as a deciding factor. Thanks for pointing it out @ivanov. 🫶

Then let's go with footprint_rectangle and the implicit convention of targeting the 2D case when naming footprints. And point out the 1D or ND support in the docstring. Shouldn't be too unexpected for an "image processing lib". 😄

footprint_cube(3, n_dim=10) is pretty unambiguous, and a more pleasant API than the technically the same footprint_rectangular((3, 3, 3, 3, 3, 3, 3, 3, 3, 3))

We explicitly chose to use an API that works for all shapes consistently, e.g., rectangles or ellipses that can't be represented with the API that you suggest. Another nice perk is, that it's consistent with NumPy's ones, zeros, etc. and rather explicit.

[lagru]

@mkcor, footprint_rect would be a way to sidestep this but how would we name the other cases, e.g., ellipsis, circle, sphere, ball? Also, I'm usually not a fan of contractions that may not be as obvious to other people that aren't already familiar with the convention.

[lagru]

I'll leave pressing the merge button to one of you, in case we have a consensus. 😉

[stefanv]

Thanks all for your inputs!

[lagru]

By the way, DIPlib uses "rectangular".