# obs_cfht unit tests are broken

XMLWordPrintable

#### Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
1
• Team:
SQuaRE

#### Description

obs_cfht has one unit test "testButler" that uses git://git.lsstcorp.org/contrib/price/testdata_cfht. 4 of the tests fail, as shown below.

In addition, testdata_cfht is huge, and the tests barely use any of it. It's worth considering making a new test repo that is smaller, or if the amount of data is small enough, move it into afwdata or obs_cfht itself.

#### Activity

Hide
Michael Wood-Vasey [X] (Inactive) added a comment -

Having no optional tests (as a long term goal) makes sense.

When I think test data for an obs_* package the only thing that comes to mind is images from that telescope+camera. These images, in general, will be large for any large format imager.

If we don't want to large test packages as part of testing an obs_* package, that would seem to rule out images.

What kind of test data would that leave?

Show
Michael Wood-Vasey [X] (Inactive) added a comment - Having no optional tests (as a long term goal) makes sense. When I think test data for an obs_* package the only thing that comes to mind is images from that telescope+camera. These images, in general, will be large for any large format imager. If we don't want to large test packages as part of testing an obs_* package, that would seem to rule out images. What kind of test data would that leave?
Hide
Russell Owen added a comment - - edited

I don't think the data needed to test obs_cfht has to be so large. I understand that natively CFHT writes single FITS files that contain all images from one focal plane, but surely it is practical to strip out all but one or two images (or, to avoid indexing issues, replace the earlier HDUs with tiny images or empty HDUs) and just fetch what remains.

Ideally the data would becomes small enough that it could be included in the obs_cfht package itself, but MegaCam has large CCDs, so that may not be practical. If obs_cfht itself is allowed to use git-lfs for some files, then I think it would be acceptable to include a few images and that would be my recommendation.

Another alternative is to generate data on the fly, but then the generating code would have to be carefully verified.

Show
Russell Owen added a comment - - edited I don't think the data needed to test obs_cfht has to be so large. I understand that natively CFHT writes single FITS files that contain all images from one focal plane, but surely it is practical to strip out all but one or two images (or, to avoid indexing issues, replace the earlier HDUs with tiny images or empty HDUs) and just fetch what remains. Ideally the data would becomes small enough that it could be included in the obs_cfht package itself, but MegaCam has large CCDs, so that may not be practical. If obs_cfht itself is allowed to use git-lfs for some files, then I think it would be acceptable to include a few images and that would be my recommendation. Another alternative is to generate data on the fly, but then the generating code would have to be carefully verified.
Hide
Michael Wood-Vasey [X] (Inactive) added a comment -

Russell Owen I hear you about the longer-term goals.

Do you have any objections to the pull request to at least just compress the files?

https://github.com/lsst/testdata_cfht/pull/2

Tim Jenness says that J Matt Peterson [X] says under git-lfs, it is helpful to compress files and that only files actually used by checked out versions get downloaded.

Show
Michael Wood-Vasey [X] (Inactive) added a comment - Russell Owen I hear you about the longer-term goals. Do you have any objections to the pull request to at least just compress the files? https://github.com/lsst/testdata_cfht/pull/2 Tim Jenness says that J Matt Peterson [X] says under git-lfs, it is helpful to compress files and that only files actually used by checked out versions get downloaded.
Hide
Russell Owen added a comment -

I have no objection to compressing the files (I assume it did saved enough space to be interesting).

Show
Russell Owen added a comment - I have no objection to compressing the files (I assume it did saved enough space to be interesting).
Hide
Michael Wood-Vasey [X] (Inactive) added a comment -

I am taking this Issues as implemented. Thanks to all for the discussion.

The outstanding unresolved issues is a larger discussion on what should obs_ packages test. I propose that we spend 6 months (perhaps 9 months include the next short+standard cycle) with new ci_ capabilities to give us a better picture of what the distinction between ci_ and obs_ tests should be and what the relationship between validation_data_ and testdata_ packages should be.

If it becomes clear that we want to revisit the testdata_* packages at that time a new ticket asking to do so would be appropriate to submit.

Show
Michael Wood-Vasey [X] (Inactive) added a comment - I am taking this Issues as implemented. Thanks to all for the discussion. The outstanding unresolved issues is a larger discussion on what should obs_ packages test. I propose that we spend 6 months (perhaps 9 months include the next short+standard cycle) with new ci_ capabilities to give us a better picture of what the distinction between ci_ and obs_ tests should be and what the relationship between validation_data_ and testdata_ packages should be. If it becomes clear that we want to revisit the testdata_* packages at that time a new ticket asking to do so would be appropriate to submit.

#### People

Assignee:
Michael Wood-Vasey [X] (Inactive)
Reporter:
Russell Owen
Reviewers:
Russell Owen
Watchers:
David Nidever [X] (Inactive), Dominique Boutigny, Hsin-Fang Chiang, Joshua Hoblitt, Michael Wood-Vasey [X] (Inactive), Paul Price, Russell Owen, Simon Krughoff, Tim Jenness