Skip to content

Conversation

@jrmullaney
Copy link
Contributor

No description provided.

@jrmullaney jrmullaney force-pushed the tickets/DM-53369 branch 2 times, most recently from e1164ce to de897f6 Compare November 26, 2025 18:42
Copy link
Contributor

@mrawls mrawls left a comment

Choose a reason for hiding this comment

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

Looks good. It appears everything you've added is very lightweight and shouldn't affect the runtime - please shout if you DON'T think that is the case (this matters a lot for AP).

self._summarize(result.exposure, summary_stat_catalog, result.background)

# Output post-subtraction background stats to task metadata:
bgIgnoreMaskPlanes = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment describing what "ignore" means here - pixels with any of these masks are excluded from the background-related statistics metric calculations. I get worried when I see "badMaskPlanes" or "ignoreMaskPlanes" out of the blue without an explicit reassurance this is just for metrics.

Also, consider making this configurable vs hard wired, in the event we want to (I don't know) add STREAK to it some day...

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the new SPIKE mask also available now, via calibrateImage? Should it be in the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for hopping on this so quickly. 👍 to adding the comment and config. Not sure about SPIKE. I did have that, but then it broke pipe_task tests in Jenkins. I've no idea why, cos it should exist.

@jrmullaney jrmullaney force-pushed the tickets/DM-53369 branch 3 times, most recently from c5359f5 to f665dd1 Compare November 26, 2025 23:39
@fred3m fred3m merged commit 54cc125 into main Nov 27, 2025
4 checks passed
@fred3m fred3m deleted the tickets/DM-53369 branch November 27, 2025 03:29
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.

4 participants