Add calib_astrometryUsed field to record sources used #67
Conversation
Also added a doWrite config field to match photoCal and measurePsf
|
But that |
| @@ -55,6 +57,11 @@ class AstrometryConfig(RefMatchConfig): | |||
| default=0.001, | |||
| min=0, | |||
| ) | |||
| doWriteOutput = pexConfig.Field( | |||
There was a problem hiding this comment.
I feel like a better name for this config parameter would be doOutputUsedFlag.
There was a problem hiding this comment.
I would like to leave this consistent with the other calib modules.
There was a problem hiding this comment.
I agree that this should be changed. This Task does not do any writing, so the name is misleading.
| @@ -147,6 +154,13 @@ def __init__(self, refObjLoader, schema=None, **kwargs): | |||
| @param[in] kwargs additional keyword arguments for pipe_base Task.\_\_init\_\_ | |||
| """ | |||
| RefMatchTask.__init__(self, refObjLoader, schema=schema, **kwargs) | |||
|
|
|||
| if self.config.doWriteOutput and not schema is None: | |||
There was a problem hiding this comment.
I'm not a fan of this code standard, but OK.
| @@ -269,6 +283,9 @@ def solve(self, exposure, sourceCat): | |||
| (iterNum, len(tryRes.matches), tryMatchDist.distMean.asArcseconds(), | |||
| tryMatchDist.distStdDev.asArcseconds())) | |||
|
|
|||
| for m in res.matches: | |||
| if self.usedKey: | |||
There was a problem hiding this comment.
probably should be explicit here: if self.usedKey is not None
|
|
||
| if self.config.doWriteOutput and not schema is None: | ||
| self.usedKey = schema.addField("calib_astrometryUsed", type="Flag", | ||
| doc="set if source was used in astrometric calibration") |
|
And we need a unittest of this functionality here as well. |
|
I have not bothered to add a unit test here. I don't think it is worth the bother, and writing any kind of intelligent unit test requires an understanding of the Task, which is well beyond the scope of this ticket. I did add one to photoCal, where it makes more sense. |
|
It looks like you've merged prematurely. |
| @@ -22,6 +22,8 @@ | |||
| from __future__ import absolute_import, division, print_function | |||
| from builtins import range | |||
|
|
|||
| import random | |||
|
|
|||
There was a problem hiding this comment.
It doesn't look like this is used.
Also added a doWrite config field to match photoCal and measurePsf