Skip to content

DM-55220: add more components when rewriting difference images - #1323

Merged
TallJimbo merged 4 commits into
mainfrom
tickets/DM-55220
Jun 30, 2026
Merged

DM-55220: add more components when rewriting difference images#1323
TallJimbo merged 4 commits into
mainfrom
tickets/DM-55220

Conversation

@TallJimbo

Copy link
Copy Markdown
Member

No description provided.

@TallJimbo
TallJimbo force-pushed the tickets/DM-55220 branch 10 times, most recently from efbaa21 to f8fdfdb Compare June 18, 2026 19:33
@TallJimbo
TallJimbo marked this pull request as ready for review June 22, 2026 14:15

@isullivan isullivan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple imports appear to be missing.

deferLoad=True, # So we can pass preserve_quantization=True as a parameter.
doc="The input image to convert.",
)
template_psf = cT.Input(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A pet peeve, which is really beyond the scope of this ticket: I really don't like the abbreviation of cT or ct for connectionTypes, since it leads to confusing errors if the code is imported in analysis_tools that have metrics with units ct. I know cT shouldn't actually conflict, so I'm not requesting you change the style here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I suspect anything in analysis_tools that uses counts should actually use electron (except maybe for HSC/DECam), because that's what we prefer for the pixel units coming out of the LSST ISR task.

inputs = butlerQC.get(inputRefs)
difference_image = inputs.pop("legacy_exposure").get(parameters={"preserve_quantization": True})
visit_summary = inputs.pop("visit_summary")
coadd_data_ids_by_uuid = {h.ref.id: h.ref.dataId for h in inputs.pop("template_coadds")}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor point, but perhaps the type template_coadds could be configurable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The dataset type name is, as is always the case for connecftions; this string is the internal name of the connection attribute.

if getattr(future_cell_coadd, "_opaque_metadata", None) is None:
future_cell_coadd._opaque_metadata = FitsOpaqueMetadata()
primary_header = future_cell_coadd._opaque_metadata.headers.setdefault(
ExtensionKey(), astropy.io.fits.Header()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see astropy.io.fits imported

primary_header.set("INSTRUME", "LSSTCam")
primary_header.set("ORIGIN", "NSF-DOE Vera C. Rubin Observatory")
primary_header.set("TELESCOP", "Rubin:Simonyi")
primary_header.set("DATE", astropy.time.Time.now().fits, "UTC date this HDU was written.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see astropy.time imported.

@TallJimbo
TallJimbo merged commit 2c8cee2 into main Jun 30, 2026
2 checks passed
@TallJimbo
TallJimbo deleted the tickets/DM-55220 branch June 30, 2026 13:32
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