XMLWordPrintable

## Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
0.5
• 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.

## Activity

Hide
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 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
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
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
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
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 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 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
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
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:
Jim Bosch
Reporter:
Jim Bosch
Reviewers:
Lauren MacArthur
Watchers:
Jim Bosch, Lauren MacArthur