DM-27169: Use FilterLabel in Exposure/ExposureInfo #446
Conversation
kfindeisen
force-pushed
the
tickets/DM-27169
branch
from
0d111f8
to
2d107d5
Compare
| @@ -13,6 +13,7 @@ CoaddInputs: lsst.obs.base.formatters.fitsGeneric.FitsGenericFormatter | |||
| VisitInfo: lsst.obs.base.formatters.fitsGeneric.FitsGenericFormatter | |||
| ApCorr: lsst.obs.base.formatters.fitsGeneric.FitsGenericFormatter | |||
| PhotoCalib: lsst.obs.base.formatters.fitsGeneric.FitsGenericFormatter | |||
| FilterLabel: lsst.obs.base.formatters.fitsGeneric.FitsGenericFormatter | |||
There was a problem hiding this comment.
It seems to me that serializing in FITS is overkill. A FilterLabel is a couple of key/value pairs isn't it? YAML or JSON seems fine for that. We could fairly trivially make the FilterFormatter work for FilterLabel and Filter since Formatters know the pytype they are formatting.
There was a problem hiding this comment.
Every component of ExposureInfo, now including FilterLabel, is held, managed, and persisted using the same framework; this makes the code much simpler and lets us avoid (or, at least, phase out) a lot of the special-case code of the old ExposureInfo. Given that FilterLabel/ExposureInfo already provides a FITS format in order to benefit from these advantages, why should we create a second format instead of reusing one that is supported out of the box? If the content of FilterLabel changes (which is quite possible, as the filter system evolves), should we need to remember to synchronize changes to two different representations?
In my view, the custom FilterFormatter was a necessary evil that standardization has made unnecessary.
kfindeisen
force-pushed
the
tickets/DM-27169
branch
from
2d107d5
to
167374e
Compare
The Exposure.filter component remains as Filter for API compatibility. Filter information will be migrated to the FilterLabel class over DM-27170 and DM-27177, then migrated back to the filter component over DM-27177 and DM-27811.
Add a link to Formatter to make that part of the documentation more prominent, and add a note about supporting the delegation.
kfindeisen
force-pushed
the
tickets/DM-27169
branch
from
02bc4e2
to
5a4d3f0
Compare
This PR adds support for
FilterLabelas a persisted form for filter information in decomposed exposures. Must be merged with lsst/afw#551.Filterremains both for backward-compatibility, and for consistency with theExposureAPI (which temporarily also hasgetFilter()andgetFilterLabel()).The plan for the
Exposureclass is to migrate code to using theFilterLabelclass (throughgetFilterLabel()) on DM-27170, removeFilterandgetFilter()and replace them with a newgetFilter()that returns aFilterLabelon DM-27177, then removegetFilterLabel()on DM-27811. I propose that the components follow the same pattern to minimize user confusion.