Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-31698

Move ConvertReferenceCatalog classes out of ingestIndex file

    XMLWordPrintable

    Details

    • Story Points:
      2
    • Sprint:
      AP F22-5 (October)
    • Team:
      Alert Production
    • Urgent?:
      No

      Description

      Once gen2 has been removed, the base class and configs for the new ConvertReferenceCatalog should be moved out of ingestIndexReferenceTask.py and into convertReferenceCatalog.py. We could potentially remove the base class entirely at that point, and just have ConvertReferenceCatalog by itself; the base class was just to let gen2/3 share most of the working code. See the note at the top of ingestIndexReferenceTask.py

      Those cannot be moved until post-gen2, because the gen2 refcat config files have an explicit check that DatasetConfig live in ingestIndexReferenceTask.

        Attachments

          Issue Links

            Activity

            Hide
            erykoff Eli Rykoff added a comment -

            In DM-35671 I did some relevant deprecations, including LoadIndexedReferenceObjectsConfig and LoadIndexedReferenceObjectsTask. I also made it so the (still poorly named; hence this ticket) preferred LoadReferenceObjectsConfig has a cal_ref_cat config field so that we can easily move from deprecated pexConfig.ConfigurableField(target=LoadIndexedReferenceObjectsTask) to undeprecated pexConfig.ConfigField(dtype=LoadReferenceObjectsConfig) (though we need to remove any overrides in configs of the deprecated cal_ref_cat field).

            Also note that currently there are tests test_loadReferenceObjects.py and test_referenceObjectLoader.py that both test different functionality of ReferenceObjectLoader. These should be consolidated on this ticket.

            Show
            erykoff Eli Rykoff added a comment - In DM-35671 I did some relevant deprecations, including LoadIndexedReferenceObjectsConfig and LoadIndexedReferenceObjectsTask . I also made it so the (still poorly named; hence this ticket) preferred LoadReferenceObjectsConfig has a cal_ref_cat config field so that we can easily move from deprecated pexConfig.ConfigurableField(target=LoadIndexedReferenceObjectsTask) to undeprecated pexConfig.ConfigField(dtype=LoadReferenceObjectsConfig) (though we need to remove any overrides in configs of the deprecated cal_ref_cat field). Also note that currently there are tests test_loadReferenceObjects.py and test_referenceObjectLoader.py that both test different functionality of ReferenceObjectLoader . These should be consolidated on this ticket.
            Hide
            Parejkoj John Parejko added a comment -

            As part of this ticket, check that the convert refcat tests still provide a proper integration test of that code and cover the important lines.

            Show
            Parejkoj John Parejko added a comment - As part of this ticket, check that the convert refcat tests still provide a proper integration test of that code and cover the important lines.
            Hide
            Parejkoj John Parejko added a comment - - edited

            Jenkins, but also note the above (cleaning up the two refloader test names, and checking that the convert code is properly covered):
            https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/37239/pipeline

            Show
            Parejkoj John Parejko added a comment - - edited Jenkins, but also note the above (cleaning up the two refloader test names, and checking that the convert code is properly covered): https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/37239/pipeline
            Hide
            Parejkoj John Parejko added a comment -

            Eli Rykoff: do you mind reviewing this, since it's related to some of the refcat work you did earlier this year? I'd suggest reviewing commits one at a time: they're pretty atomic, and the final one in meas_algorithms ("consolidate refcat tests") is primarily moving tests from one file into two others, and doesn't involve any new code (though I tweaked a couple docstrings).

            As far as I can tell, the convert tests still have appropriate coverage (it's just not obvious because the multiprocessing pool masks what lines are run to the coverage app).

            New Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/37463/pipeline

            Show
            Parejkoj John Parejko added a comment - Eli Rykoff : do you mind reviewing this, since it's related to some of the refcat work you did earlier this year? I'd suggest reviewing commits one at a time: they're pretty atomic, and the final one in meas_algorithms ("consolidate refcat tests") is primarily moving tests from one file into two others, and doesn't involve any new code (though I tweaked a couple docstrings). As far as I can tell, the convert tests still have appropriate coverage (it's just not obvious because the multiprocessing pool masks what lines are run to the coverage app). New Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/37463/pipeline
            Show
            Parejkoj John Parejko added a comment - Post-review/rebase jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/37469/pipeline

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              Parejkoj John Parejko
              Reviewers:
              Eli Rykoff
              Watchers:
              Eli Rykoff, John Parejko
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.