Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-48783: Add illumination correction support to SubtractBackgroundTask and SourceDetectionTask #412

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

erykoff
Copy link
Contributor

@erykoff erykoff commented Feb 18, 2025

This PR also adds some unit tests for SubtractBackgroundTask and the background re-estimation in SourceDetectionTask.

@@ -291,6 +293,8 @@ def detectFootprints(self, exposure, doSmooth=True, sigma=None, clearMask=True,
Results from preliminary detection pass.
(`lsst.pipe.base.Struct`)
"""
if backgroundToPhotometricRatio is not None:
raise RuntimeError("DynamicDetectionTask does not support backgroundToPhotometricRatio.")
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth it to add a config validate check for this, too.

do anything.
backgroundToPhotometricRatio : `lsst.afw.image.Image`, optional
Image to convert background-flattened image to
photometric-flattened image (and back).
Copy link
Member

Choose a reason for hiding this comment

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

When you fixed the implementation on the later commit, I don't think the docs had to change, because I don't feel like "apply" implies either "multiply" or "divide". But I also think we want the docs to talk about going from a photometric-flattened image to a backtround-flattened one and back, since that's what the context actually does. How about:

Image to multiply a photometrically-flattened image by to obtain a background-flattened image.

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.

2 participants