Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: fgcmcal, jointcal, meas_algorithms, pipe_tasks
-
Labels:
-
Story Points:4
-
Epic Link:
-
Sprint:AP F22-3 (August)
-
Team:Alert Production
-
Urgent?:No
Description
Eli Rykoff noted on DM-35670 that we need the name of the loaded refcat to apply colorterms, but in gen2 that was carried through as a config parameter on LoadIndexedReferenceObjectsTask. ReferenceObjectLoader doesn't currently have any knowledge of the name of the loaded refcat, but the calling Task does in the Connection name. The easiest fix for this is to add a required name kwarg to ReferenceObjectLoader.__init__(). Tasks that use a refcat loader would then pass the Connection name in runQuantum() when they create the loader.
Attachments
Issue Links
Activity
Eli Rykoff: are you ok to review this? 8 PRs, but they're all short. I believe I caught all uses of this in the lsst org (including ones in faro that aren't actually tested!), and I didn't see any in lsst-dm. This is technically an API change since `name` is now a required arg, but I don't think anyone tries to use ReferenceObjectLoader directly, since they have to get the datarefs and htm ids first.
- https://github.com/lsst/meas_algorithms/pull/290
- https://github.com/lsst/pipe_tasks/pull/716
- https://github.com/lsst/analysis_tools/pull/27
- https://github.com/lsst/faro/pull/139
- https://github.com/lsst/fgcmcal/pull/90
- https://github.com/lsst/jointcal/pull/229
- https://github.com/lsst/analysis_drp/pull/50
- https://github.com/lsst/atmospec/pull/24
- https://github.com/lsst/ap_pipe/pull/124
- https://github.com/lsst/drp_pipe/pull/38
- https://github.com/lsst/obs_decam/pull/231
- https://github.com/lsst/obs_lsst/pull/420
- https://github.com/lsst/obs_subaru/pull/433
There are some docstring and usage comments on the meas_algorithms and pipe_tasks PRs. Otherwise looks good.
Sorry for the trouble, Eli Rykoff: after your review, I reworked some things and added a few more PRs to remove the `ref_dataset_name` usage everywhere I could, and replace it with this new `name` parameter if necessary. Please take another look, and note that there are several more PRs (all linked above).
This looks good. Thanks for going through and cleaning up all the deprecated usage of ref_dataset_name!
I've merged everything except the atmospec PR, while I wait for feedback from Merlin Fisher-Levine; I think there's an already-broken gen2 refcat usage in processStar.py, but the tests don't run it.
As I said on the PR, look like atmospec was already broken, so go ahead and merge, and we'll work on fixing it up later.
Jenkins run with ci_hsc and ci_imsim: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/37209/pipeline