DM-42126: Remove ignored detectors exposures from being processed in SubtractBrightStarsTask #869
Conversation
bazkiaei
force-pushed
the
tickets/DM-42126
branch
2 times, most recently
from
0c19e69 to
cdf106d
Compare
| def detInRegions(self, inputs): | ||
| """Determine whether the input exposure's detector is in the region(s) | ||
| where the extended PSF model(s) is(are) available. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| inputs : `dict` | ||
| Dictionary containing the inputs to the task, including | ||
| `inputExposure` and `inputExtendedPsf`. |
There was a problem hiding this comment.
Rather than a single global inputs dict here, I think it would make more sense to split this into two required inputs: inputExposure and inputExtendedPsf. This will make it clear exactly what inputs from the input dict are required, what their purposes are, and prevent accidental usage of any other input.
| @@ -364,6 +372,32 @@ def run( | |||
|
|
|||
| return subtractorExp, invImages, badStamps | |||
|
|
|||
| def detInRegions(self, inputs): | |||
There was a problem hiding this comment.
Is this intended to be used globally, outside this file? If not, we might consider renaming this to _detInRegions.
| availableDets = [ | ||
| det | ||
| for detList in inputs["inputExtendedPsf"].detectors_focal_plane_regions.values() | ||
| for det in detList.detectors | ||
| ] | ||
| if inputs["inputExposure"].detector.getId() in availableDets: |
There was a problem hiding this comment.
I'm a fan of full words where possible. I think that helps readability in the future for other developers contributing to this package and maintaining it.
So here for example, availableDetectors for availableDets, detector for det and detectors for detList (or detectorList, if you want to avoid the subsequent detectors.detectors potential confusion). However, this is not a major comment, and I'll defer to you if you feel the way you have it now is more legible in general.
bazkiaei
force-pushed
the
tickets/DM-42126
branch
from
cdf106d to
a27369e
Compare
bazkiaei
force-pushed
the
tickets/DM-42126
branch
from
1fddd47 to
f350890
Compare
No description provided.