Add unit tests for photutils/aper_core.py

[cdeil]

As can be seen here unit test coverage for the photutils/aper_core.py module is only at 49%: https://coveralls.io/files/389247253

If someone has time to add tests for some of the methods that do computations before the 0.1 release next week, that would be great (but if not, this should not hold up the release).

@bsipocz ?

[bsipocz]

@cdeil: I'll look at this in the weekend. Those test were there during the summer, so I only need to dig them up.

On 12 December 2014 at 20:32, Christoph Deil notifications@github.com wrote:

As can be seen here unit test coverage for the photutils/aper_core.py module is only at 49%: https://coveralls.io/files/389247253

If someone has time to add tests for some of the methods that do computations before the 0.1 release next week, that would be great (but if not, this should not hold up the release).

@bsipocz https://github.com/bsipocz ?

— Reply to this email directly or view it on GitHub https://github.com/astropy/photutils/issues/201.

[cdeil]

Thanks for looking at this! (It could also be that there's something wrong with the coverage report ... I didn't look at all whether it matches the tests that are in place.)

[bsipocz]

It looks like, that the coverage is off as the SkyCircularAperture, and thus all of the SkyCoord related code are tested only in a test flagged with remote-data. However there was also a bug in SkyElliptical, and it indeed wasn't covered with any tests.

The other issue is that a significant amount of the code is about making the plots. What kind of meaningful test can cover those?

[cdeil]

If a lot of unit tests currently depend on remote data that's bad ... really there's very few (or none) cases where a remote dataset is better for a unit test. Instead we should use the dataset examples like make_4gaussians_image that are always available.

@bsipocz Do you have time to change the tests to use no remote data before the 0.1 release next week? If not I can do this on Tuesday.

Concerning testing the plotting, I think we should test this like this: https://github.com/astropy/astropy/pull/2139/files#diff-2170332cf8c89cd3e19baea05248e711R1177 I.e. run the code but don't to asserts on the generated output for now. This is better than no tests at all and we can make an issue for the 0.2 release that we should extend the tests to check the generated output images.

[bsipocz]

How difficult is to put a mock header with wcs into those generaged examples? The remote data test is for the SkyAperture classes, thus some wcs is needed to make the tests sensible.

[cdeil]

It's very easy to make a mock header by passing a dict to the header constructor, see .e.g. gammapy.image.make_header.

The best solution would be to put this in astropy.image and use it here, but I never found the time to put it in. For now, I think an OK solution for photutils would be to copy this function as a private _make_header function into photutils.datasets and use it to make mock headers.

@larrybradley You wrote the functions like make_4gaussians_image ... would you be OK with changing them to return an ImageHDU? Or would you prefer some other solution to make mock headers available for those images?

[cdeil]

Oh, and @bsipocz ... thanks for all the PRs helping get photutils in shape for 0.1!

[larrybradley]

@cdeil We could also return (image, header) from those functions, but I'm OK with returning an ImageHDU. Should I do this or @bsipocz are you working on this?

[bsipocz]

@larrybradley - I've almost finished it already, and will add the commits to #203 tomorrow.

On 17 December 2014 at 20:01, Larry Bradley notifications@github.com wrote:

@cdeil https://github.com/cdeil We could also return (image, header) from those functions, but I'm OK with returning an ImageHDU. Should I do this or @bsipocz https://github.com/bsipocz are you working on this?

— Reply to this email directly or view it on GitHub https://github.com/astropy/photutils/issues/201#issuecomment-67383734.