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

assembleCoadd broken

    Details

      Description

      A recent update to assembleCoadd to bring over changes to do clipped coadds breaks coadd generation. There are two specific problems.

      1. There is an infinite recursion because of SafeClipAssembleCoaddTask calling its own constructor in the __init__ method.
      2. The overridden assemble method does not adhere to the original assemble call signature, so when the default run method is called by ParseAndRun, it raises an exception.

      Additionally I find the flow fairly confusing as the overridden assemble method is called by the default run method which then calls the default assemble method on the parent class.

        Attachments

          Issue Links

            Activity

            Hide
            krughoff Simon Krughoff added a comment -

            John Swinbank Thanks for looking. Both of those tickets will help. Did you capture the need for a refactor anywhere?

            Show
            krughoff Simon Krughoff added a comment - John Swinbank Thanks for looking. Both of those tickets will help. Did you capture the need for a refactor anywhere?
            Hide
            rowen Russell Owen added a comment -

            This looks great.

            I have two small requests for the doc string of SafeClipAssembleCoaddTask.assemble:

            • document that args and *kwargs are ignored
            • correct the spelling of "vists" to "visits"
            Show
            rowen Russell Owen added a comment - This looks great. I have two small requests for the doc string of SafeClipAssembleCoaddTask.assemble : document that args and *kwargs are ignored correct the spelling of "vists" to "visits"
            Hide
            krughoff Simon Krughoff added a comment -

            Merged prematurely, so made the doc changes on master.

            Show
            krughoff Simon Krughoff added a comment - Merged prematurely, so made the doc changes on master.
            Hide
            swinbank John Swinbank added a comment -

            Simon Krughoff: There's no ticket which explicitly requests that this task code be refactored. We have DM-3581, an epic, which covers re-working our top level tasks post HSC-merge; I've added a note there that we should specifically consider AssembleCoadd when we do so.

            Show
            swinbank John Swinbank added a comment - Simon Krughoff : There's no ticket which explicitly requests that this task code be refactored. We have DM-3581 , an epic, which covers re-working our top level tasks post HSC-merge; I've added a note there that we should specifically consider AssembleCoadd when we do so.
            Hide
            swinbank John Swinbank added a comment -

            Looks like this was merged many years ago.

            Show
            swinbank John Swinbank added a comment - Looks like this was merged many years ago.

              People

              • Assignee:
                krughoff Simon Krughoff
                Reporter:
                krughoff Simon Krughoff
                Reviewers:
                Russell Owen
                Watchers:
                Hsin-Fang Chiang, John Swinbank, Paul Price, Russell Owen, Simon Krughoff
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel