DM-25424: Convert Defect to IsrCalib #216
Conversation
tests/test_cr.py
Outdated
| @@ -120,8 +121,11 @@ def testDetection(self): | |||
| # Mask known bad pixels | |||
| # | |||
| measAlgorithmsDir = lsst.utils.getPackageDir('meas_algorithms') | |||
| badPixels = algorithms.Defects.readText(os.path.join(measAlgorithmsDir, | |||
| "policy", "BadPixels.ecsv")) | |||
| defectTable = Table.read(os.path.join(measAlgorithmsDir, "policy", "BadPixels.ecsv")) | |||
There was a problem hiding this comment.
This now looks funny because it's divorcing that test file from the Defects implementation but we don't really have any use for that file any more. For the purposes of these tests we'd be better off with a helper function that returns a list of two bounding boxes and we wouldn't even need the file. I don't like this Table.read code now appearing in three separate test files.
There was a problem hiding this comment.
I've added a helper function to lsst.meas.algorithms.testUtils to generate the common list of badPixels, and have removed policy (which only contained that file).
tests/test_cr.py
Outdated
| badPixels = [algorithms.Defect(lsst.geom.Box2I(lsst.geom.Point2I(row['x0'], row['y0']), | ||
| lsst.geom.Extent2I(row['width'], row['height']))) | ||
| for row in defectTable] | ||
|
|
||
| # did someone lie about the origin of the maskedImage? If so, adjust bad pixel list | ||
| if self.XY0.getX() != self.mi.getX0() or self.XY0.getY() != self.mi.getY0(): |
There was a problem hiding this comment.
I am worried that this is some weird left over from an old bug. I see it copied in another file.
There was a problem hiding this comment.
The XY0 appears to be related to hard coded definitions in setUp:
https://github.com/lsst/meas_algorithms/blob/tickets/DM-25424/tests/test_cr.py#L65
https://github.com/lsst/meas_algorithms/blob/tickets/DM-25424/tests/test_measure.py#L187
Although the interior of those blocks has changed recently (~2018), the blocks themselves are very old (~2009).
There was a problem hiding this comment.
Does the branch ever trigger or does it always trigger? I'm trying to understand why it might only sometimes trigger in a test.
There was a problem hiding this comment.
test_measure.py never appears to trigger the block to do the shift.
test_cr.py does trigger, but that appears to be that after setting self.XY0 = lsst.geom.PointI(32, 2) to define the start of the box, the origin of self.mi is set to (0, 0) (Line 69). Removing this line prevents the block with the shifts from triggering. Without applying the shift, the test still finds the correct number of cosmic rays (1076), but the output list is shifted by the same (32, 2) relative to the list retaining the origin update on Line 70.
I had thought that this was related to the debug statement on line 147 (also hardcoded not to run), but the lines generated there are also shifted by self.mi.getX0/Y0.
This is now looking like one lie (setting the origin on Line 69) requiring one shift (the block starting on line 125; this prevents the CR finding failing with too many CR pixels, as the defects are in the wrong place) followed by a second shift (line 150) to ensure the debug code works.
I believe my solution will be to remove the block in test_measure.py, remove the lie and the first shift, and keep the debug shift in place.
branch tickets/DM-25424-transfer. The C++ Defect type is staying here, as the interpolation and cosmic ray code depend on it. The tests remaining here exercise that code, using a simple list to hold multiple Defect objects.
czwa
force-pushed
the
tickets/DM-25424
branch
from
1b323ed
to
5ba01fa
Compare
czwa
force-pushed
the
tickets/DM-25424
branch
from
5ba01fa
to
d7d8502
Compare
Remove Defects from meas_algorithms. This code is moving to ip_isr on
branch tickets/DM-25424-transfer. The C++ Defect type is staying
here, as the interpolation and cosmic ray code depend on it. The
tests remaining here exercise that code, using a simple list to hold
multiple Defect objects.