DM-21308: Replace doApplyUberCal with doApplyExternal configs #340
Conversation
| retrieved from the `calexp`. When `config.doApplyExternalSkyWcs` is `True`, | ||
| the astrometric calibration is taken from `config.externalSkyWcsName` with | ||
| the `name_wcs` dataset. Otherwise, the astrometric calibration is taken | ||
| from the `calexp`. |
There was a problem hiding this comment.
If you can take a moment to numpydoc-ify this along the way, I would give you a gold star ⭐️
| else: | ||
| photoCalib = exposure.getPhotoCalib() | ||
|
|
||
| if self.config.doApplyExternalSkyWcs: | ||
| skyWcs = dataRef.get(f"{self.config.externalSkyWcsName}_wcs") | ||
| exposure.setWcs(skyWcs) |
There was a problem hiding this comment.
I am belatedly seeing the comment in the obs_subaru HSC makeCoaddTempExp.py which states "This was copy-pasted from MakeCoaddTempExpTask, and extra step added to check whether an additional mask is needed." BOOO. The docstring further admits it is a hack.
This explains why I was confusing myself thinking I had already reviewed this code 😠
Anyway, assuming this code duplication is an independent problem I cannot control, and will be fixed in DM-20074 per the comment in obs_subaru, please see my review comments in obs_subaru, where I suggest changes in literally identical code.
tests/test_makeCoaddTempExp.py
Outdated
| # a "jointcal_photoCalib" calibration to return | ||
| self.jointcalPhotoCalib = lsst.afw.image.PhotoCalib(1e-6, 1e-8) | ||
| # An ubercalibrated photoCalib calibration to return | ||
| self.uberPhotoCalib = lsst.afw.image.PhotoCalib(1e-6, 1e-8) |
There was a problem hiding this comment.
Please call this external rather than uber throughout the tests
|
Please also consider squashing some of the commits here. |
erykoff
force-pushed
the
tickets/DM-21308
branch
from
0143a96
to
0d54cf3
Compare
The doApplyUberCal config has been deprecated in favor of doApplyExternalPhotoCalib and doApplyExternalSkyWcs. These names are consistent with the updates to validate_drp in DM-21950, and allow different external calibrations (jointcal, fgcmcal, fgcmcal_tract) to be used, and for external photometric and astrometric calibrations to be applied separately. Finally, useMeasMosaic has been properly deprecated.
erykoff
force-pushed
the
tickets/DM-21308
branch
from
0d54cf3
to
bde4216
Compare
No description provided.