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

Remove unused code from coadd_utils

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: coadd_utils
    • Labels:
      None

      Description

      Remove the Coadd class, any supporting code, and the makeBitMask function.

        Attachments

          Issue Links

            Activity

            Show
            swinbank John Swinbank added a comment - Hey Nate Lust , do you have time for a small review? PR here: https://github.com/lsst/coadd_utils/pull/22 Jenkins here: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29316/pipeline
            Hide
            swinbank John Swinbank added a comment -

            Hey Nate, no rush, but do you have an ETA on this review? Thanks!

            Show
            swinbank John Swinbank added a comment - Hey Nate, no rush, but do you have an ETA on this review? Thanks!
            Hide
            nlust Nate Lust added a comment -

            Some changes need to be made, see GitHub

            Show
            nlust Nate Lust added a comment - Some changes need to be made, see GitHub
            Hide
            swinbank John Swinbank added a comment -

            Hey Nate Lust — thanks for the review!

            Per comments on GitHub:

            • pipe_drivers actually has its own implementation of TractDataIdContainer; it's not relying on coadd_utils;
            • Although TractDataIdContainer is imported in qa_explorer, it doesn't seem to actually be used. I've made a PR against qa_explorer to remove all the unused imports, including this one.
            • Lauren MacArthur points out that TractDataIdContainer is also used by pipe_analysis, but urges us not to block this work on sorting that out.

            Given the above, do you have any further concerns here? There are no changes on the coadd_utils PR, but I'd be much obliged if you could give the new one against qa_explorer a quick sanity check.

            Show
            swinbank John Swinbank added a comment - Hey Nate Lust — thanks for the review! Per comments on GitHub: pipe_drivers actually has its own implementation of TractDataIdContainer ; it's not relying on coadd_utils; Although TractDataIdContainer is imported in qa_explorer, it doesn't seem to actually be used. I've made a PR against qa_explorer to remove all the unused imports, including this one. Lauren MacArthur points out that TractDataIdContainer is also used by pipe_analysis, but urges us not to block this work on sorting that out. Given the above, do you have any further concerns here? There are no changes on the coadd_utils PR, but I'd be much obliged if you could give the new one against qa_explorer a quick sanity check.
            Hide
            nlust Nate Lust added a comment -

            Looks good to me, pending jenkins tests, I think you are good to mrege

            Show
            nlust Nate Lust added a comment - Looks good to me, pending jenkins tests, I think you are good to mrege
            Hide
            swinbank John Swinbank added a comment -

            Since there are no changes to coadd_utils since Jenkins #29316, and since qa_explorer isn't covered by Jenkins, I'm going to go ahead and merge. Thanks Nate!

            Show
            swinbank John Swinbank added a comment - Since there are no changes to coadd_utils since Jenkins #29316, and since qa_explorer isn't covered by Jenkins, I'm going to go ahead and merge. Thanks Nate!
            Hide
            lauren Lauren MacArthur added a comment -

            Just for closure, this has been addressed for pipe_analysis in DM-11974.

            Show
            lauren Lauren MacArthur added a comment - Just for closure, this has been addressed for pipe_analysis in DM-11974 .
            Hide
            swinbank John Swinbank added a comment -

            Thank you!

            Show
            swinbank John Swinbank added a comment - Thank you!

              People

              • Assignee:
                swinbank John Swinbank
                Reporter:
                swinbank John Swinbank
                Reviewers:
                Nate Lust
                Watchers:
                John Swinbank, Lauren MacArthur, Nate Lust
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel