# Refactor ForcedPhotImageTask (and children) per RFC-352

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
2
• Sprint:
DRP F18-3
• Team:
Data Release Production

#### Description

Implementing RFC-352  prepping for the supertask conversion in two stages. DM-2639 changed the TaskRunner to call a Task's runDataRef method.  During code review Russell Owen points out that ForcedPhotImageTask (and its children ForcedPhotCcdTask and ForcedPhotCoaddTask  would be simple to refactor.

#### Activity

Hide
Christopher Waters added a comment -

Hopefully a quick review.

Small refactor of ForcedPhotImageTask to add a run method. ForcedPhotCcd and ForcedPhotCoadd will both inherit this method.

Show
Christopher Waters added a comment - Hopefully a quick review. Small refactor of ForcedPhotImageTask to add a run method. ForcedPhotCcd and ForcedPhotCoadd will both inherit this method.
Hide
Russell Owen added a comment -

Thank for doing this. I had a few small requested changes on github.

Show
Russell Owen added a comment - Thank for doing this. I had a few small requested changes on github.
Hide

Only because I'm a watcher on this ticket: This implementation looks incomplete because you're just passing the DataRef to run.

You can call the IdFactory in runDataRef and pass it to run. Do any of the children's attachFootprints()  actually need the DataRef? You might be able to change that API.

Show
Yusra AlSayyad added a comment - Only because I'm a watcher on this ticket: This implementation looks incomplete because you're just passing the DataRef to run. You can call the IdFactory in runDataRef and pass it to run. Do any of the children's attachFootprints()  actually need the DataRef? You might be able to change that API.
Hide

OK, I see the answer to my question is yes a child needs to fetch the heavy footprints from disk. So maybe everything down and including the call to attachFootprints() should be part of runDataRef and run just does

  self.measurement.run(measCat, exposure, refCat, refWcs,   if self.config.doApCorr:  self.applyApCorr.run(  catalog=measCat,  apCorrMap=exposure.getInfo().getApCorrMap()  )  self.catalogCalculation.run(measCat) 

Show
Yusra AlSayyad added a comment - OK, I see the answer to my question is yes a child needs to fetch the heavy footprints from disk. So maybe everything down and including the call to attachFootprints() should be part of runDataRef and run just does self.measurement.run(measCat, exposure, refCat, refWcs, if self.config.doApCorr: self.applyApCorr.run( catalog=measCat, apCorrMap=exposure.getInfo().getApCorrMap() ) self.catalogCalculation.run(measCat)
Hide
Russell Owen added a comment -

Good catch. Sorry I missed that and I agree with Yusra AlSayyad. I'll update my comments on github.

Show
Russell Owen added a comment - Good catch. Sorry I missed that and I agree with Yusra AlSayyad . I'll update my comments on github.
Hide
Christopher Waters added a comment - - edited

That makes sense to isolate the DataRef from run().

I'm running a new version through Jenkins to make sure I have everything correct in the new code, and will update the branch once that passes.

Show
Christopher Waters added a comment - - edited That makes sense to isolate the DataRef from run(). I'm running a new version through Jenkins to make sure I have everything correct in the new code, and will update the branch once that passes.

#### People

Assignee:
Christopher Waters
Reporter:
Reviewers:
Russell Owen
Watchers:
Christopher Waters, Russell Owen, Yusra AlSayyad