Removal of shift_val from EPSFModel and EPSFBuilder

[Onoddil]

Currently both EPSFModel and EPSFBuilder carry the keyword shift_val, which is likely unnecessary. EPSFModel carries it around for one instance in EPSFBuilder where it is passed to a function -- but EPSFBuilder also carries what is essentially the same variable (as this is the only place it is used). It should therefore be stripped from EPSFModel as an unneeded variable and its call replaced with the one in EPSFBuilder.

That said, however, EPSFBuilder doesn't need to carry shift_val either. The variable is only used when the recentering_func is centroid_epsf (which it is by default, admittedly). If we bumped centroid_epsf from a function (made as such initially to be the same as other functions, like centroid_com) to a class, we could have something like

class CentroidEPSF():
    def __init__(self, shift_val=0.5):
        self.shift_val = shift_val
    
    def __call__(data, mask=None, oversampling=4):
        # basically do exactly as is done now in the function, but in the call instead
        centroid_epsf(data, mask=mask, oversampling=oversampling, shift_val=self.shift_val)

(although obviously I assume you'd just move the code from centroid_epsf into the __call__ function, I was just being lazy with the code above). Then EPSFBuilder just needs its recentering_func=centroid_epsf replaced with recentering_func=CentroidEPSF(), and several checks for self.recentering_func == centroid_epsf replaced with some isinstance checks and it should all work as it was, removing shift_val from both ePSF framework objects.

It does add one extra line of call to current users' codes, where they need to make some_epsf_centroid = CentroidEPSF(shift_val=2) to then pass recentering_func=some_epsf_centroid into EPSFBuilder, which isn't going to be backwards compatible (by default, if they don't add the initialisation line, their shift_val will change to CentroidEPSF's default... but then if they don't do anything to the code they'd get an unexpected keyword presumably), but it should be cleaner code.

[Onoddil]

While not strictly necessary, and only tangentially related to the above (hence the separate comment), this could also be a good time to consider more stringent checks on shift_val and oversampling. Currently, to calculate the integer grid point shift (i.e., how many grid points over from the grid point representing the middle of the detector) at which to compute the asymmetry metric x_shiftidx is a simple integer rounding of shift_val multiplied by oversampling. However, a more thorough check of whether that value is sufficiently close to an integer might not be a bad idea, as that could produce unexpected results. It is probably fine most of the time, and I don't know if in this instance it's better to silently fix the user's mistake and potentially mislead them vs raising more warnings/errors and risking losing more important warnings in the noise.

[Onoddil]

(Could be merged with #996, while the codebase is being fixed and/or updated)

[eteq]

After some out-of-band discussion, @Onoddil and I are thinking now that this should be a class like this:

class CentroidEPSF:
    def __init__(self, det_pix_dist=0.5, oversampling=None, grid_offset=None):
        ...

Then in the builder, any of the None's get replaced by their correct value for whatever the builder keywords are. That removes the need to do any validation of the combinations of oversampling and grid_offset in the builder itself, or the call to the function (which should improve performance).

This would be as a follow-on to #989 which adds the grid_offset and so on.

In this plan, the key idea is that None means "the user wants the builder to take care of it", so then the various parameters should be implemented as properties, where the setters call a _validate method, which does validation only once all of the parameters are set to non-None values.

[eteq]

Also, the shift_val in the builder should be deprecated in favor of det_pix_dist. This has some subtleties, so here are the user experiences I think we want:

>>> EPSFBuilder(...)
(uses the defaults without any special messages)
>>> EPSFBuilder(..., centroid=CentroidEPSF(det_pix_dist=0.4))
(uses the requested det_pix_dist without any special messages)
>>> EPSFBuilder(..., shift_val=0.4)
Deprecationwarning: shift_val should be passed into CentroidEPSF as det_pix_dist instead of the builder
(then goes ahead and does it by forcing CentroidEPSF.det_pix_dist to whatever shift_val is set to)
>>> EPSFBuilder(..., centroid=CentroidEPSF(det_pix_dist=0.4), shift_val=0.4)
Deprecationwarning: shift_val should be passed into CentroidEPSF as det_pix_dist instead of the builder (i.e., exact same as above, doesn't need to be a special case)
>>> EPSFBuilder(..., centroid=CentroidEPSF(det_pix_dist=0.4), shift_val=0.6)
ValueError: shift_val and CentroidEPSF.det_pix_dist are inconsistent. Use det_pix_dist only because shift_val is deprecated.