# Remove setXY0 from image classes

XMLWordPrintable

#### Details

• Type: RFC
• Resolution: Unresolved
• Component/s:
• Labels:
None

#### Description

Our code currently allows the image, mask, and variance planes of a MaskedImage to have different XY0 origins, and hence different bounding boxes:

 >>> mi = MaskedImage(4, 3) >>> image = mi.getImage() >>> image.setXY0(1, 2) >>> mi.getXY0() Point2I(1, 2) >>> mi.getVariance().getXY0() Point2I(0, 0) 

The same problem also affects all of the spatially-aware non-Image objects attached to Exposure (e.g. Wcs and Psf: if the user calls setXY0 on the Exposure object, none of these objects are modified or notified, and hence they remain in the old coordinate system.

Implementing DM-7565 will add new problems of the same type.

The simplest solution by far is to remove setXY0 (at least as a public method). This does not affect the primary use of XY0 (making the coordinate system of a subimage the same as its parent), which happens at construction and is hence safe since all-subobjects can be set consistently.

Instead, I propose adding a shiftedTo method to Image, Mask, and MaskedImage, which will return a view of the image with xy0 modified. For convenience and clarity I also propose adding shiftedBy, which takes an Extent2I and applies a relative offset.

I do not propose adding these methods to Exposure, because implementing them correctly there would require both implementing a shiftedTo method on all of Exposure's components. I think it's likely that any code that currently calls Exposure.setXY0 can and should be rewritten to avoid doing so.

The shiftedTo and shiftedBy names follow this recommendation in the C++ style guide, which I'd like to see us start using more often (especially in afw.geom) to avoid confusion between in-place and return-new operations.

Whether setXY0 is kept as a private method is left as an implementation detail (but if it is, it should of course be renamed to to _setXY0).

#### Activity

Hide
Jim Bosch added a comment - - edited

I think adding an xy0 keyword argument to some constructors may be a good idea, but I'm not convinced we should add it to the ones just taking dimensions, because we already have one that takes a bbox that already covers that case:

  bb = fp.getBBox()  if mask:  im = afwImage.MaskedImageF(bb)  else:  im = afwImage.ImageF(bb)    fp.insert(im) 

Show
Jim Bosch added a comment - - edited I think adding an xy0 keyword argument to some constructors may be a good idea, but I'm not convinced we should add it to the ones just taking dimensions, because we already have one that takes a bbox that already covers that case: bb = fp.getBBox() if mask: im = afwImage.MaskedImageF(bb) else: im = afwImage.ImageF(bb) fp.insert(im)
Hide
Jim Bosch added a comment -

We will add the ability to set xy0 explicitly upon construction to any image class constructor that does not already have that capability.

Show
Jim Bosch added a comment - Adopting with the following adjustments: We will add the ability to set xy0 explicitly upon construction to any image class constructor that does not already have that capability.
Hide
Russell Owen added a comment - - edited

Good. But I hope we will only have xy0 as an argument in cases where the dimensions are known; if both xy0 and dimensions can be specified in the argument list then we should use bbox as the argument.

Show
Russell Owen added a comment - - edited Good. But I hope we will only have xy0 as an argument in cases where the dimensions are known; if both xy0 and dimensions can be specified in the argument list then we should use bbox as the argument.
Hide
Robert Lupton added a comment -

I don't understand Russel's comment. You sometimes need to set XY0 to override the value from e.g. a BBox.

Show
Robert Lupton added a comment - I don't understand Russel's comment. You sometimes need to set XY0 to override the value from e.g. a BBox.
Hide
Jim Bosch added a comment -

I think this just a misunderstanding; to clarify, this signature:

 Image(Box2I const & bbox); 

does not need a new xy0 argument, because you can already control the new xy0 exactly. But this one does:

 Image(Image const & other, Box2I const & bbox, ImageOrigin parent, bool deep); 

because the Box here is used to define the region from other we want to copy/view, and parent only gives us limited control over the output xy0.

Show
Jim Bosch added a comment - I think this just a misunderstanding; to clarify, this signature: Image(Box2I const & bbox); does not need a new xy0 argument, because you can already control the new xy0 exactly. But this one does: Image(Image const & other, Box2I const & bbox, ImageOrigin parent, bool deep); because the Box here is used to define the region from other we want to copy/view, and parent only gives us limited control over the output xy0 .

#### People

Assignee:
Jim Bosch
Reporter:
Jim Bosch
Watchers:
Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen, Robert Lupton, Russell Owen, Tim Jenness