XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
4
• Sprint:
AP F22-3 (August)
• Team:
• 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.

#### Activity

Hide
John Parejko added a comment - - edited
Show
John Parejko added a comment - - edited Jenkins run with ci_hsc and ci_imsim: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/37209/pipeline
Hide
John Parejko added a comment - - edited

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.

Show
John Parejko added a comment - - edited 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
Hide
Eli Rykoff added a comment -

There are some docstring and usage comments on the meas_algorithms and pipe_tasks PRs. Otherwise looks good.

Show
Eli Rykoff added a comment - There are some docstring and usage comments on the meas_algorithms and pipe_tasks PRs. Otherwise looks good.
Hide
John Parejko added a comment - - edited

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).

Show
John Parejko added a comment - - edited 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).
Hide
Eli Rykoff added a comment -

This looks good. Thanks for going through and cleaning up all the deprecated usage of ref_dataset_name!

Show
Eli Rykoff added a comment - This looks good. Thanks for going through and cleaning up all the deprecated usage of ref_dataset_name !
Hide
John Parejko added a comment -

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.

Show
John Parejko added a comment - 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.
Hide
Merlin Fisher-Levine added a comment -

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.

Show
Merlin Fisher-Levine added a comment - 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.

#### People

Assignee:
John Parejko
Reporter:
John Parejko
Reviewers:
Eli Rykoff
Watchers:
Eli Rykoff, Ian Sullivan, Jim Bosch, John Parejko, Merlin Fisher-Levine, Nate Lust