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

Revert accidental rename of id->objectId in ForcedPhotCoaddTask

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_base
    • Labels:
      None
    • Story Points:
      0.5
    • Epic Link:
    • Sprint:
      DRP F16-6
    • Team:
      Data Release Production

      Description

      The "id" column in coadd forced photometry outputs is being renamed to "objectId", due to some code intended to add an "objectId" column to CCD level forced photometry being accidentally applied.

        Attachments

          Activity

          Hide
          lauren Lauren MacArthur added a comment -

          Ah, you're right. I'm getting myself confused with the config outputs, where the overrides occur (and which even have any effect at all!). In my config output file from recent multibandDriver.py, I have these two entries:

          # Mapping of reference columns to source columns
          config.forcedPhotCoadd.copyColumns={'id': 'objectId', 'parent': 'parentObjectId', 'deblend_nChild': 'deblend_nChild'}
          # Mapping of reference columns to source columns
          config.forcedPhotCoadd.measurement.copyColumns={'id': 'objectId', 'parent': 'parentObjectId'}
          

          So your update would override the former, right. Would that even have had an effect given that the copyColumns happens in forcedMeasurement.py?

          Show
          lauren Lauren MacArthur added a comment - Ah, you're right. I'm getting myself confused with the config outputs, where the overrides occur (and which even have any effect at all!). In my config output file from recent multibandDriver.py , I have these two entries: # Mapping of reference columns to source columns config.forcedPhotCoadd.copyColumns={'id': 'objectId', 'parent': 'parentObjectId', 'deblend_nChild': 'deblend_nChild'} # Mapping of reference columns to source columns config.forcedPhotCoadd.measurement.copyColumns={'id': 'objectId', 'parent': 'parentObjectId'} So your update would override the former, right. Would that even have had an effect given that the copyColumns happens in forcedMeasurement.py ?
          Hide
          jbosch Jim Bosch added a comment -

          Yeah, my previous change was incorrect. I've now changed that, and I hope it's all working, but I'm waiting for Jenkins to confirm that. I've added a test (in pipe_tasks) to check that.

          Show
          jbosch Jim Bosch added a comment - Yeah, my previous change was incorrect. I've now changed that, and I hope it's all working, but I'm waiting for Jenkins to confirm that. I've added a test (in pipe_tasks) to check that.
          Hide
          jbosch Jim Bosch added a comment -

          Lauren MacArthur, I believe I've fixed everything, and I think the new test demonstrates that. And I hope I answered your questions well enough above. I think this is now ready for another look.

          Show
          jbosch Jim Bosch added a comment - Lauren MacArthur , I believe I've fixed everything, and I think the new test demonstrates that. And I hope I answered your questions well enough above. I think this is now ready for another look.
          Hide
          lauren Lauren MacArthur added a comment -

          This looks good to me now, and it's great that you added a unittest (and got rid of the temporary skip since I last looked!).

          Feel free to merge.

          Show
          lauren Lauren MacArthur added a comment - This looks good to me now, and it's great that you added a unittest (and got rid of the temporary skip since I last looked!). Feel free to merge.
          Hide
          jbosch Jim Bosch added a comment -

          I thought I might have been able to fix that temporary-skip mistake before you noticed, but I guess not. Merged to master.

          Show
          jbosch Jim Bosch added a comment - I thought I might have been able to fix that temporary-skip mistake before you noticed, but I guess not. Merged to master.

            People

            Assignee:
            jbosch Jim Bosch
            Reporter:
            jbosch Jim Bosch
            Reviewers:
            Lauren MacArthur
            Watchers:
            Jim Bosch, Lauren MacArthur
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.