DM-47084: getOverlappingExposures docs and other cleanups #346
Conversation
parejkoj
force-pushed
the
tickets/DM-47084
branch
from
c330797 to
c1cbb56
Compare
python/lsst/ip/diffim/getTemplate.py
Outdated
| """Return a data structure containing the coadds that overlap the | ||
| specified bbox projected onto the sky, and a corresponding data | ||
| structure of their dataIds. | ||
| These are the appropriate inputs to this task's `run` method. | ||
|
|
||
| The spatial index in the registry has generous padding and often |
There was a problem hiding this comment.
I think I don't understand what "spatial index" here is referring to. If this is one of the inputs maybe its worth explaining which?
There was a problem hiding this comment.
Please have another look at this text: I reworked it a bit to hopefully be more clear?
|
|
||
| The spatial index in the registry has generous padding and often | ||
| supplies patches near, but not directly overlapping the detector. | ||
| Filters inputs so that we don't have to read in all input coadds. | ||
| This filters the inputs so that we don't have to read in all | ||
| possibly-matching coadds. | ||
|
|
There was a problem hiding this comment.
By "read in" does it mean load all the images in the input list (to check for overlap)? Perhaps this stems from my confusion over "spatial index in the registry" (does this mean butler registry, i.e. is referring to how all the images are indexed in the database?
|
|
||
| Parameters | ||
| ---------- | ||
| inputs : `dict` of task Inputs, containing: | ||
| - coaddExposures : `list` \ | ||
| [`lsst.daf.butler.DeferredDatasetHandle` of \ | ||
| `lsst.afw.image.Exposure`] | ||
| Data references to exposures that might overlap the detector. | ||
| Data references to exposures that might overlap the desired | ||
| region. |
There was a problem hiding this comment.
I think I was sending it a dictionary containing a list of data references? I find info on the format needed to be useful but I defer to developers the correct way to document these things
There was a problem hiding this comment.
As noted in the docstring, this is a list of datarefs. See the type listing above this line.
RFC-1052 is about refactoring this method (and run) to take specific arguments instead of a dict.
| - wcs : `lsst.afw.geom.SkyWcs` | ||
| Template WCS onto which to resample the coaddExposures. | ||
| Template WCS onto which the coadds will be resampled, . | ||
|
|
There was a problem hiding this comment.
an extra comma at the end of the -wcs explanation
There was a problem hiding this comment.
Also separate question: does this function only work on coadds, or is it possible this would be used for calexp files also (i.e. perhaps more generic reference to input images rather than specifically coadds would make sense?)
There was a problem hiding this comment.
Coadds are organized per tract/patch, other exposures are not. That tract/patch organization is a requirement here.
| name=BaseSkyMap.SKYMAP_DATASET_TYPE_NAME, | ||
| dimensions=("skymap", ), | ||
| storageClass="SkyMap", | ||
| ) | ||
| # TODO DM-31292: Add option to use global external wcs from jointcal | ||
| # Needed for DRP HSC | ||
| coaddExposures = pipeBase.connectionTypes.Input( | ||
| doc="Input template to match and subtract from the exposure", | ||
| doc="Coadds that may overlap the desired region, as possible inputs to the template." | ||
| " Will be restricted to those that directly overlap the projected bounding box.", | ||
| dimensions=("tract", "patch", "skymap", "band"), |
There was a problem hiding this comment.
That's necessary, so that there is a space after the period on the previous line.
parejkoj
force-pushed
the
tickets/DM-47084
branch
3 times, most recently
from
7b194d7 to
4b4ca87
Compare
parejkoj
force-pushed
the
tickets/DM-47084
branch
from
4b4ca87 to
b74ca14
Compare
It's clearer to pull out the `inputs` from the loop, and we only need to test that the wcs is not None once, not in the loop.
parejkoj
force-pushed
the
tickets/DM-47084
branch
from
b74ca14 to
65f4026
Compare
No description provided.