None value for MoffatPSF.n_pix and MoffatPSF.peak

[bazkiaei]

While I use these initial values for MoffatPSF and PixellatedPSF (just like what we do in test_psf.py):

psf_moffat = MoffatPSF(FWHM=1 / 30 * u.arcminute, shape=4.7)

psf_data = np.array([[0.0, 0.0, 0.1, 0.0, 0.0],
                     [0.0, 0.3, 0.7, 0.4, 0.0],
                     [0.1, 0.8, 1.0, 0.6, 0.1],
                     [0.0, 0.2, 0.7, 0.3, 0.0],
                     [0.0, 0.0, 0.1, 0.0, 0.0]])
psf_pixellated = PixellatedPSF(psf_data=psf_data,
                     psf_sampling=1 * u.arcsecond / u.pixel,
                     psf_centre=(2, 2),
                     oversampling=10,
                     pixel_scale=(2 / 3) * u.arcsecond / u.pixel)

There are proper values for psf_pixellated.n_pix and psf_pixellated.peak but for psf_moffat.n_pix and psf_moffat.peak the values are None.

I found out that pixel_scale for psf_moffat is None value as well and that might be the reason why the other two n_pix and peak are None as they seems to be related to pixel_scale as follow lines in psf.py:

266        if self.pixel_scale:
267            self._update_model()

And it is strange, before implementing pytest.mark.parametrize, how test_psf.py does work while these values are None?! Does it mean, for instance here:

def test_peak(psf_moffat):
    assert psf_moffat.peak == 0.7134084656751443 / u.pixel

while the test is comparing None to a number, it just does noting and passes?

[AnthonyHorton]

The fact that the pixel_scale attribute of psf_moffat is None is indeed the reason why the n_pix and peak attributes are also None. Until you tell the MoffatPSF what pixel scale to use it can't calculate those other properties.

The change in behaviour compared to before is probably to do with the fact that parametrize isn't handling fixtures properly, and is creating a new MoffatPSF for each test instead of returning a reference to the same object for all the tests. That tells us that the now failing tests were relying on an earlier test to set the pixel scale. We shouldn't be doing that anyway ('non-hermetic tests').

And no, when None is compared to a number it will always fail.

[lspitler]

@AnthonyHorton is the solution to just add pixel_scale to the MoffatPSF call in test_psf.py? e.g. psf_moffat = MoffatPSF(FWHM=1 / 30 * u.arcminute, shape=4.7, (2 / 3) * u.arcsecond / u.pixel)

@bazkiaei I believe @AnthonyHorton 's comments about the test order relates to the same problem I had in a test I wrote: https://github.com/AstroHuntsman/gunagala/pull/25#discussion_r220454510 . Here was my fix: https://github.com/AstroHuntsman/gunagala/pull/25/commits/f115e54657a9c650e212e1cb04f07b893eb914a4

@bazkiaei I hadn't realised during our discussion today that this issue is a subset of Issue #26. I guess this can fall under the PR you just opened.

[AnthonyHorton]

Setting the pixel scale when creating the MoffatPSF in the psf_moffat fixture as you suggest would fix the immediate problem. The only real downside to doing that I can see is that it does prevent testing that it returns all the Nones that it should before the pixel scale is set. That would be easily fixed though by having another fixture that returns a MoffatPSF without pixel scale just for that test.

The root cause of all this though is that pytest.parametrize is not behaving as we'd expect when we try to give it fixtures as the parameter values. A more thorough look at the pytest documentation, or code, might reveal a neater, simpler fix to our test issues.

[AnthonyHorton]

Closing as MoffatPSF is behaving as intended. This was a bug in the tests, since addressed.