Tickets/dm 16501 #72
Tickets/dm 16501 #72
Conversation
|
Looks like this is now behind master (Remove the unused MultiBandDataIdContainer has already been merged: 9411d8c (just in case you hadn't noticed). |
| @@ -192,6 +150,10 @@ def __init__(self, butler=None, schema=None, refObjLoader=None, reuse=tuple(), * | |||
| self.measureCoaddSources.schema)) | |||
| self.makeSubtask("forcedPhotCoadd", refSchema=afwTable.Schema( | |||
| self.mergeCoaddMeasurements.schema)) | |||
| if self.config.fakesAdded: | |||
| self.coaddType = "fakes_" + self.config.coaddName | |||
There was a problem hiding this comment.
Consider string formatting
| @@ -45,6 +45,7 @@ class SkyCorrectionConfig(Config): | |||
| doBgModel = Field(dtype=bool, default=True, doc="Do background model subtraction?") | |||
| doSky = Field(dtype=bool, default=True, doc="Do sky frame subtraction?") | |||
| binning = Field(dtype=int, default=8, doc="Binning factor for constructing focal-plane images") | |||
| fakesAdded = Field(dtype=bool, default=False, doc="Have fakes been added?") | |||
There was a problem hiding this comment.
See comment in another repo about the name fakes added, I think a different name like fakesPresent or something like that is closer to the meaning you had in mind, but I dont have very strong feeling on this
| @@ -113,14 +114,19 @@ def runDataRef(self, expRef): | |||
| if DEBUG: | |||
| extension = "-%(visit)d.fits" % expRef.dataId | |||
|
|
|||
| if self.config.fakesAdded: | |||
There was a problem hiding this comment.
This code block should not be duplicated twice, consider moving it to init and assigning to self, either with the name calexpType or something else if you are worried about that name being used elsewhere
| @@ -157,10 +163,14 @@ def runDataRef(self, tractPatchRefList, butler, selectIdList=[]): | |||
| self.log.info("Non-empty tracts (%d): %s" % (len(tractPatchRefList), | |||
| [patchRefList[0].dataId["tract"] for patchRefList in | |||
| tractPatchRefList])) | |||
| if self.config.fakesAdded: | |||
There was a problem hiding this comment.
Dont have this twice, consider making it class level
|
|
||
| def setDefaults(self): | ||
| self.makeCoaddTempExp.select.retarget(NullSelectImagesTask) | ||
| self.assembleCoadd.select.retarget(NullSelectImagesTask) | ||
| self.assembleCoadd.doWrite = False | ||
|
|
||
| def validate(self): | ||
| if self.fakesAdded: |
There was a problem hiding this comment.
I don't think validate should be used to assign anything itself, it should only be used to check if options were set in a consistent way.
| @@ -35,13 +35,19 @@ class CoaddDriverConfig(Config): | |||
| doc="Run detection on the coaddition product") | |||
| detectCoaddSources = ConfigurableField( | |||
| target=DetectCoaddSourcesTask, doc="Detect sources on coadd") | |||
| fakesAdded = Field(dtype=bool, default=False, doc="Have fakes been added?") | |||
There was a problem hiding this comment.
This needs a better doc, and consider a different name, see comments on related ticket
sr525
force-pushed
the
tickets/DM-16501
branch
from
0e3cc72
to
4deb452
Compare
No description provided.