Most of the Psf methods that can compute something at a particular point in an image allow for the actual position argument to be omitted, in which case an "average" position is used by default. See many of the methods in Psf.h This pattern can easily lead to problems when either:
- the programmer forgets to specify a position (see https://github.com/lsst/meas_extensions_gaap/pull/19/files#diff-a2b9e86cf219f9d08340e128bd919d674908cb94a15eeb8231c4936e544df26eL121)
- the programmer assumes that the computation is independent of position (https://github.com/lsst/meas_extensions_shapeHSM/pull/33/files#diff-ded2fd089f49b8ea4563875ff5d65ca1b978c723e9d35bc5d38c726c6419dd47L200).
In cases where the position really is intended to be the average position, it's easy enough to query the Psf object that the programmer already has access to, e.g.:
Since this pattern appears to be dangerous and unnecessary, I propose to deprecate the no-position-argument interface in each of:
and their python equivalents.
(I haven't tried, but I'm guessing we could implement this in c++ by swapping the default parametered methods for overloads).
Second, Psf::getAveragePosition() has a virtual default implementation, which just returns Point2D(). It is up to derived classes to override this if required, which should be the case for all non-constant Psf models. However, I'm pretty sure I didn't do this right in meas_extensions_piff. So part (b) of this RFC is to add a Psf::_averagePosition member variable and require constructors to initialize it. This is effectively already done in a number of derived classes, e.g.,
Making it mandatory will prevent any future Psf derived classes from naively inheriting a default implementation.