ROI implementation of getting the center position is inconsistent

[pllim]

• glue.core.roi.RectangularROI.center()
• glue.core.roi.CircularROI.get_center()
• glue.core.roi.EllipticalROI.xc, glue.core.roi.EllipticalROI.yc

Even when base class documentation said center() should work. 🤷

[pllim]

p.s. Looks like CircularROI.get_center() is just a shortcut to get CircularROI.xc and CircularROI.yc. But there is no RectangularROI.xc and RectangularROI.yc.

[dhomeier]

RectangularROI.center() should return self.xmin + self.width() / 2, self.ymin + self.height() / 2 (which is xc, yc for the rectangle, so to speak) – what is not working there? EllipticalROI.get_center() and PolygonalROI.center() are being implemented in #2235; thanks for pointing out the inconsistent naming. Will probably need discussion if the former can be renamed or aliased.

[pllim]

what is not working there?

I don't think anything wasn't working. I just couldn't use the API consistently when my code need to handle different shapes.

Out of scope here but something to consider is how to calculate rectangle center when you add in ability to rotate it. (Basically calculating center for any arbitrary polygon shape since rotated rectangle is basically subset of that.)

[dhomeier]

Out of scope here but something to consider is how to calculate rectangle center when you add in ability to rotate it. (Basically calculating center for any arbitrary polygon shape since rotated rectangle is basically subset of that.)

Yes, that's discussed in https://github.com/glue-viz/glue/pull/2235#issuecomment-931280123 – basically xc, yc as well as width() and height() remain invariant under rotations around that centre. For general polygons, this holds for a centre defined as (np.mean([vx, vy], axis=1).