Skip to content

Conversation

@laurenam
Copy link
Contributor

No description provided.

@laurenam laurenam force-pushed the tickets/DM-53647 branch 3 times, most recently from a0b69e6 to 41c058a Compare January 22, 2026 02:50
@laurenam laurenam requested a review from kherner January 22, 2026 02:57
return bg

def _computeBrightDetectionMask(self, maskedImage, convolveResults):
def _computeBrightDetectionMask(self, maskedImage, convolveResults, maxBrightDetectionIter=15):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to leave the default undefined here if it's getting pulled from the config? I'm just wondering if one can avoid having to set it in two places (here and line 127) if it ever changes in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point! Since it's essentially a private function anyway, I switched it so just use the config (in line with the other uses in this function), and I renamed the config to more closely follow the established conventions here (i.e. moved Max to the end of the name). I also reduced the default to 10 (which is still likely overkill, but this should be a total edge case situation, so shouldn't cost us much).

This guards against potentially very long (or infinite) running
loops.

Also adds a unittest check that forces entry into this loop to make
sure it iterates just two times, having set the full images as having
the DETECTION mask set, and checking that it fails at trying to lay
down sky sources.
@laurenam laurenam merged commit 1cc47fb into main Jan 27, 2026
4 checks passed
@laurenam laurenam deleted the tickets/DM-53647 branch January 27, 2026 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants