DM-28597: Fix colorterm/photoCal filterLabel confusion #174
Conversation
| @@ -756,7 +756,7 @@ def _load_data(self, inputSourceTableVisit, inputVisitSummary, associations, joi | |||
| The filter bands of each input dataset. | |||
| """ | |||
| oldWcsList = [] | |||
| bands = [] | |||
| filters = [] | |||
There was a problem hiding this comment.
Here and below, I think that filters is actually filterLabels? If so, I think you should be explicit in the name because filter is such an overloaded term at the moment. Instead of these are physical filters this should be physicalFilters, but I think they're labels.
There was a problem hiding this comment.
Argh, github won't let me add a comment above that the docstring is incorrect, it isn't returning bands it's returning something else (filterLabels or physicalFilters).
There was a problem hiding this comment.
@kfindeisen suggested that I leave these as "filter/filters" because that is what the RFC process will eventually settle on everywhere else (once the deprecation period is over), and there was no deprecation danger here. filter in the pipelines code will mean filterLabel pretty much everywhere.
See the "Transition" section here: https://community.lsst.org/t/migrating-from-afw-image-filter-to-filterlabel/4591
| @@ -887,7 +887,7 @@ def loadData(self, dataRefs, associations, jointcalControl): | |||
| """Read the data that jointcal needs to run. (Gen2 version)""" | |||
| visit_ccd_to_dataRef = {} | |||
| oldWcsList = [] | |||
| bands = [] | |||
| filters = [] | |||
There was a problem hiding this comment.
Same here for the filterLabel (I think) and this method doesn't have a proper docstring.
There was a problem hiding this comment.
If it's ok, I'm not going to bother adding a docstring since this is purely gen2.
| @@ -925,10 +925,10 @@ def _prep_sky(self, associations, bands): | |||
| self.log.info(f"Data has center={center} with radius={radius.asDegrees()} degrees.") | |||
|
|
|||
| # Determine a default filter band associated with the catalog. See DM-9093 | |||
| defaultBand = bands.most_common(1)[0][0] | |||
| self.log.debug("Using '%s' filter band for reference flux", defaultBand) | |||
| defaultFilter = filters.most_common(1)[0][0] | |||
There was a problem hiding this comment.
I think this is a defaultFilterLabel?
* Colorterms are now explicitly keyed on physical label, while the refcat loader uses band, so we need to pass around FilterLabel objects. * No need to return band in _build_ccdImage() since the caller will always have it via JointcalInputData. * cleanup tests that passed filters around
parejkoj
force-pushed
the
tickets/DM-28597
branch
from
9992299
to
bb8d892
Compare
Now that obs_cfht is using FilterDefinitionCollections, we have to use that to reset them, and we need to protect any uses of `createTwoFakeCcdImages`, since those create a Camera.
No description provided.