DM-14580: Create tests for BestSeeingWcsSelectImagesTask #223
Conversation
| @@ -400,7 +400,7 @@ def runDataRef(self, dataRef, coordList, makeDataRefList=True, | |||
| if selectDataList is None: | |||
| selectDataList = [] | |||
|
|
|||
| result = super().runDataRef(dataRef, coordList, makeDataRefList, selectDataList) | |||
| result = super().runDataRef(dataRef, coordList, makeDataRefList=True, selectDataList=selectDataList) | |||
There was a problem hiding this comment.
The default makeDataRefList is True in both WcsSelectImagesTask and BestSeeingWcsSelectImagesTask, so it's not clear to me why you need to explicitly put it here also.
There was a problem hiding this comment.
We need to allow BestSeeingWcsSelectImagesTask to have makeDataRefList = False. But the BestSeeingWcsSelectImagesTask relies on having the dataRefList returned by the subclassed WcsImagesTask, so we have to set makeDataRefList = True in the call to super.
| Parameters | ||
| ---------- | ||
| mockFWHMs : function, iterable, exeception, or `None` | ||
| Returns FWHM of mocked images |
There was a problem hiding this comment.
I don't think this actually returns anything, so this should probably be, e.g., "Creates". If I'm wrong and it does return something, please use the proper Returns numpydoc syntax.
| self.calexp = unittest.mock.Mock(spec=lsst.afw.image.ExposureD) | ||
| self.calexp.getPsf.return_value.computeShape.return_value.getDeterminantRadius.side_effect \ | ||
| = mockDeterminantRadii() | ||
| self.dataId = "I'm a mock!" |
There was a problem hiding this comment.
This is my new favorite dataId 💯
|
|
||
| self.selectList = [self.selectStruct] * len(self.mockFWHMs) | ||
|
|
||
| def testConfigNImagesGTZero(self): |
There was a problem hiding this comment.
I know many of these are super self explanatory, but please add a short docstring to each anyway.
| class BestSeeingWcsSelectImagesTest(lsst.utils.tests.TestCase): | ||
|
|
||
| def setUp(self): | ||
|
|
| result = task.runDataRef(self.dataRef, self.coordList, | ||
| selectDataList=self.selectList, makeDataRefList=False) | ||
| self.assertIsNone(result.dataRefList) | ||
|
|
There was a problem hiding this comment.
These test cases are marginally interesting, but what I'd really like to see is a slightly more realistic case where more than Nmax images meet the good seeing criterion but have different seeings. For example, if there are 10 images with acceptable FWHM values, but they aren't all the same, and Nmax = 3, I think the desired behavior would be for the selector to choose the three with the lowest FWHM values. This requires testing more than just the length of the resulting exposure list, but rather what FWHM values and/or dataIds made it into the list.
| result = task.runDataRef(self.dataRef, self.coordList, selectDataList=self.selectList) | ||
| self.assertEqual(len(result.exposureInfoList), 3) | ||
|
|
||
| def testSelectDefaultMakeDatarRefListIsFalse(self): |
There was a problem hiding this comment.
Typo in the name (your data is (are?) pirate flavored... Datarrrrrr...)
ebellm
force-pushed
the
tickets/DM-14580
branch
2 times, most recently
from
fbe46e9
to
8603310
Compare
ebellm
force-pushed
the
tickets/DM-14580
branch
from
8603310
to
9b444f3
Compare
No description provided.