Tickets/dm 7976 #81
Tickets/dm 7976 #81
Conversation
| from lsst.afw.math import ChebyshevBoundedField | ||
| from lsst.pipe.tasks.coaddInputRecorder import CoaddInputRecorderTask | ||
|
|
||
| SaveCoadd = False # if True then save coadd even if test passes (always saved if a test fails) |
There was a problem hiding this comment.
Could this lead to problems if the test fails for some new reason, and then confusingly continues to fail because the committed test data file has been overwritten by something incorrect?
There was a problem hiding this comment.
I don't think so, because the coadd is always written before being read.
|
|
||
|
|
||
| class CoaddInputsTestCase(lsst.utils.tests.TestCase): | ||
| def setUp(self): |
There was a problem hiding this comment.
Need an empty line before this one, right?
| """Make a simple mock exposure suitable to put in a coadd | ||
|
|
||
| The metadata is set, but not the pixels | ||
| """ |
There was a problem hiding this comment.
Reads line a method doc, not a class doc; maybe "Helper class for making simple mock exposures..." I also think you need an empty line after it.
| else: | ||
| print("SaveCoadd true; saved coadd as: %r" % (coaddPath,)) | ||
|
|
||
| def xtestReadV1Coadd(self): |
| if version > 1: | ||
| self.assertEqual(visit.getVisitInfo(), expInfo.getVisitInfo()) | ||
| else: | ||
| self.assertFalse(visit.getVisitInfo().getDate().isValid()) |
There was a problem hiding this comment.
Just a reminder that if you do change the code in afw to make this an empty pointer, you'll want to change this line too (to check that getVisitInfo() is None).
There was a problem hiding this comment.
Yes, I did realize that. And it improves the test.
|
|
||
| def assertCoaddInputsOk(self, coaddInputs, version): | ||
| self.assertIsNotNone(coaddInputs) | ||
| visitTable = coaddInputs.visits |
There was a problem hiding this comment.
If you're going to test just one of these, it should be ccds, not visits; most of these quantities in the visits table come from the coaddTempExp, which is probably better than setting them to None but might still be confusing in some cases (e.g. "the visit WCS is the same as the coadd WCS"). The ccds table is what corresponds to what is in a calexp, and that's what most people will want to look at and hence what we want to make sure doesn't regress.
There was a problem hiding this comment.
Fair enough. It's a delicate point because I'm not actually making these from coaddTempExp, but directly from exposures with their own VisitiInfo. Presumably they will be identical, so I'll apply the same test to both.
r-owen
force-pushed
the
tickets/DM-7976
branch
from
8a3ac4a
to
fd4c46d
Compare
It was not saving Psf, Wcs, ValidPolygon and VisitInfo. Add a unit test for this, including a test for reading version 1 CoaddInput/ExposureTable.
r-owen
force-pushed
the
tickets/DM-7976
branch
from
fd4c46d
to
eff8975
Compare
No description provided.