DM-35496: Update coord columns per reevaluated WCS #698
Conversation
| @@ -673,6 +673,9 @@ def addCalibColumns(self, catalog, exposure, exposureIdInfo, **kwargs): | |||
| newCat = afwTable.SourceCatalog(schema) | |||
| newCat.extend(catalog, mapper=mapper) | |||
|
|
|||
| if self.config.doReevaluateSkyWcs: | |||
There was a problem hiding this comment.
It took me a little while to track down the difference between doApplyExternalSkyWcs and doReevaluateSkyWcs--would it be worth clarifying in the docs that the former updates the wcs of the exposure while the latter updates the coordinates of the input catalog, if I am interpreting correctly?
There was a problem hiding this comment.
doReevaluateSkyWcstells it whether or not to update the WCS in the source table at all.- IFF that's true,
doApplyExternalSkyWcstells it whether to usecalexp.wcsorjointcal. - IFF that's that's true,
useGlobalExternalSkyWcstells it whether to use jointcal-global or jointcal-tract.
doApplyExternalSkyWcs+useGlobalExternalSkyWcs here means the thing as all the other Tasks that decide whether to get their WCS from the calexp, jointcal tract-mode, or jointcal global-mode (i.e. makeWarp, ForcedPhotCcd and faro). I find the control structure that you find in every __init__ of Configs that have a doApplyExternalSkyWcs super confusing, hate it, and hope that one day when you make gbdes an option that you find a good replacement for doApplyExternalSkyWcs+useGlobalExternalSkyWcs everywhere.
I opted for consistency.
to help, I'll add a line to the docstring of doApplyExternalSkyWcs "Used if and only if doReevaluateSkyWcs"
| @@ -673,6 +673,9 @@ def addCalibColumns(self, catalog, exposure, exposureIdInfo, **kwargs): | |||
| newCat = afwTable.SourceCatalog(schema) | |||
| newCat.extend(catalog, mapper=mapper) | |||
|
|
|||
| if self.config.doReevaluateSkyWcs: | |||
| afwTable.updateSourceCoords(exposure.wcs, newCat) | |||
There was a problem hiding this comment.
Do you need to have a corresponding statement for self.config.doReevaluatePhotoCalib?
There was a problem hiding this comment.
No, and I'll drop an inline comment on why not.
yalsayyad
force-pushed
the
tickets/DM-35496
branch
from
92f5af8
to
99b8c19
Compare
No description provided.