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

Create placeholder AP DPDDifying task

    Details

      Description

      The new API of ap_association in DM-15588 assumes that all SourceCatalogs fed into are calibrated data products with column names close to that of the DPDD and Ppdb schema.

      This ticket creates a task to convert outputs from ip_diffim into a usable format for ap_association to be use in association and set to the PPDB to be written.

        Attachments

          Activity

          Hide
          yusra Yusra AlSayyad added a comment -

          Still haven't found the time for a line review. Some comments after skimming it.

          1) Just recently approved by the change control board, we're providing fluxes in NanoJanskys rather than NanoMaggies.
          2) Looks like you have two types of operations to columns now: Copy and InstFluxToMaggies. Is the design that for each new type of operation on a column, you add a new config parameter with a list of those columns? What happens if you have an operation that requires two columns. e.g. XYErr to Ra/DecErr?

          You call it a placeholder because you need something to load into the ppdb now. For that purpose this looks fine. I worry about extensibility in the future.

          Show
          yusra Yusra AlSayyad added a comment - Still haven't found the time for a line review. Some comments after skimming it. 1) Just recently approved by the change control board, we're providing fluxes in NanoJanskys rather than NanoMaggies. 2) Looks like you have two types of operations to columns now: Copy and InstFluxToMaggies. Is the design that for each new type of operation on a column, you add a new config parameter with a list of those columns? What happens if you have an operation that requires two columns. e.g. XYErr to Ra/DecErr? You call it a placeholder because you need something to load into the ppdb now. For that purpose this looks fine. I worry about extensibility in the future.
          Hide
          swinbank John Swinbank added a comment -

          You call it a placeholder because you need something to load into the ppdb now. For that purpose this looks fine. I worry about extensibility in the future.

          For what it's worth, I agree with that concern, but also with your reading of the priorities. We're waiting to see what comes out of DM-14568 before doing anything too substantial here.

          Show
          swinbank John Swinbank added a comment - You call it a placeholder because you need something to load into the ppdb now. For that purpose this looks fine. I worry about extensibility in the future. For what it's worth, I agree with that concern, but also with your reading of the priorities. We're waiting to see what comes out of DM-14568 before doing anything too substantial here.
          Hide
          yusra Yusra AlSayyad added a comment -

          See notes in github

          Show
          yusra Yusra AlSayyad added a comment - See notes in github
          Hide
          cmorrison Chris Morrison added a comment -

          Hey Yusra AlSayyad, take a my new commits and let me know if I've missed anything. I'll rebase once I hear back from ya. Jenkins job submitted.

          Show
          cmorrison Chris Morrison added a comment - Hey  Yusra AlSayyad , take a my new commits and let me know if I've missed anything. I'll rebase once I hear back from ya. Jenkins job submitted.
          Show
          cmorrison Chris Morrison added a comment - https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29008/pipeline/
          Hide
          yusra Yusra AlSayyad added a comment -

          OK to squash and merge

           

          Show
          yusra Yusra AlSayyad added a comment - OK to squash and merge  

            People

            • Assignee:
              cmorrison Chris Morrison
              Reporter:
              cmorrison Chris Morrison
              Reviewers:
              Yusra AlSayyad
              Watchers:
              Chris Morrison, John Swinbank, Yusra AlSayyad
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel