Working integration test for CFHT data. #14
Conversation
Major refactor of testing code to use new JointcalTestBase, pulling most of the work out of the old testJointcal.py (which is now testJointcal_lsstSim.py, and which now uses the base class). New testJointcal_cfht uses this base class. Also improved plotting code to generate per-visit quiver plots. testJointcal_lsstSim doesn't use the parseAndRun code, because the lsstSim data is missing some necessary metadata. Can't solve that until the new metadata class is built and the twinkles data is reprocessed. The test code still works fine, though. Improved some docstrings in jointcal.py. Left in the commented-block that used the old StarSelector: I think we can remove it when we have a few more robust source flags to describe good astrometric sources. Jointcal.run() now returns a struct with the modified dataRefs and the old WCSs. JointcalRunner now works correctly with parseAndRun, fixing DM-6915. Also made it return a Struct for testing the results. Added coaddName back to JointcalConfig (parseAndRun needs it).
| task.run(*args) | ||
| result = task.run(*args) | ||
| if self.doReturnResults: | ||
| return pipeBase.Struct(result = result) |
There was a problem hiding this comment.
No spaces around = if it's all on one line (e.g., see line 59).
|
There are multiple orthogonal changes in this commit, e.g., adding comments, changing docs, etc. These should be separate commits. |
| @@ -135,7 +156,8 @@ def _build_ccdImage(self, dataRef, associations, jointcalControl): | |||
|
|
|||
| goodSrc = self.sourceSelector.selectSources(src) | |||
|
|
|||
| # TODO: NOTE: Leaving the old catalog and comparison code in while we | |||
| # ------------------ | |||
| # TODO: Leaving the old catalog and comparison code in while we | |||
There was a problem hiding this comment.
Code should not be left commented out. I know you have philosophical disagreements, but I think you should submit to the way things are done until you have explicit approval to change them.
There was a problem hiding this comment.
I just submitted an RFC. I'll note that I couldn't find any explicit statement about this in the python coding standards.
There was a problem hiding this comment.
I did add a note about the conditions under which the code would be removed.
There was a problem hiding this comment.
I've now wrapped this code in if useOldStarSelector: with the note above it describing when it will be removed.
| old_rel_total, old_abs_total, new_rel_total, new_abs_total) | ||
|
|
||
| # So one can muck-about with things after plotting... | ||
| import ipdb |
There was a problem hiding this comment.
Wouldn't work on my machine (no ipdb module). Can you use just pdb instead?
There was a problem hiding this comment.
ipdb is to pdb what ipython is to python: gives tab-completion, smart history, magic methods, and other handy features. pip installable. Highly recommended for debugging python. That said, I think I'll change this to be a "plt.show" instead.
There was a problem hiding this comment.
You should be able to run it now: I have both plt.show() (might be ignored by a test suite) and pdb.set_trace() (helpful for looking at other values). I've also tweaked the histogram plot a bit based on your hipchat comment.
Better documentation of relative and absolute error thresholds. New and improved plotting code for interpreting results. Pulled various blocks into short helper functions. Began moving test files to _ variable names.
Forgot to commit updated cfht tests.
Major refactor of testing code to use new JointcalTestBase, pulling most of the
work out of the old testJointcal.py (which is now testJointcal_lsstSim.py, and
which now uses the base class). New testJointcal_cfht uses this base class. Also
improved plotting code to generate per-visit quiver plots.
testJointcal_lsstSim doesn't use the parseAndRun code, because the lsstSim data
is missing some necessary metadata. Can't solve that until the new metadata
class is built and the twinkles data is reprocessed. The test code still works
fine, though.
Improved some docstrings in jointcal.py. Left in the commented-block that used
the old StarSelector: I think we can remove it when we have a few more robust
source flags to describe good astrometric sources.
Jointcal.run() now returns a struct with the modified dataRefs and the old WCSs.
JointcalRunner now works correctly with parseAndRun, fixing DM-6915. Also made
it return a Struct for testing the results.
Added coaddName back to JointcalConfig (parseAndRun needs it).