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

Shape Naming and Non-Centered Rectangles #10

Open
Squareys opened this issue Sep 7, 2015 · 2 comments
Open

Shape Naming and Non-Centered Rectangles #10

Squareys opened this issue Sep 7, 2015 · 2 comments

Comments

@Squareys
Copy link
Contributor

Squareys commented Sep 7, 2015

Hi everybody!

I noticed the following naming inconsistency:

  • RectangleShape has a span in every direction and is therefore a hypercube and centered
  • CenteredRectangleShape has no non-centered counterpart.

In general, there are no non-centered RectangleShapes and RectangleShape could be easily implemented over a fitting span parameter for CenteredRectangleShape.

This will probably be hard to do because of API breaks, but I would suggest the following:

Rename CenteredRectangleShape to (Hyper)RectangleShape and make it uncentered, then do the centering and the Cube shapes over constructor parameters or subclasses (Hyper)CubeShape and Centered*Shape.

As far as I can tell, the current implementation of RectangleNeighborhood (aka. CenteredRectangleNeighborhood) is flexible enough to be easily adapted to an uncentered implementation.
Benefits: Straight-forward naming and code reuse for all Rectangle-based shapes.

Please tell me what you think and if I missed something.
Greetings, Squareys.

@tibuch
Copy link
Contributor

tibuch commented Sep 7, 2015

I would like that :)

About the naming:

Maybe we should call it Orthotope. Rectangle implies 2D and Cube implies 3D but such a shape can have up to n dimensions.

@tpietzsch
Copy link
Member

@Squareys I agree. I think it would be best to have only a single RectangleShape and with different constructors. We have to keep in mind that Shape has no fixed dimensionality yet. (That may not have been the best design decision and may change once we unify everything into imglib2-roi, but for now that's how it is). CenteredRectangleShape currently does that wrong.

I would propose that the RectangleShape is constructed either with an Interval.
If the Interval has dimension n and the Shape is instantiated with dimension m, then:
If m <= n, just use the first m dimensions of the interval. If m > n, then replicate the last dimension of the interval for all higher dimensions.
There could be a shortcut constructor that just takes int min, int max, specifying a 1-d interval.

The "centered" constructors (or sublass) would take a int[] span (or int span as shortcut for the 1-d span) and then create the interval from that.

@Squareys If you need it and want to do it, please go ahead. Otherwise I'll put it on my list...

@tibuch I don't like Orthotope so much because nobody will look for the Shape under this name. Except @axtimwalde possibly :) ... Wikipedia suggests https://en.wikipedia.org/wiki/Hyperrectangle that Hyperrectangle and Box are also possible names. I think both would be better. I like Box, actually, although it also kind of implies 3D.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants