DM-28323: Make scarlet the default deblender #455
Conversation
fred3m
changed the title
| @@ -2,7 +2,7 @@ description: Multiband | |||
| tasks: | |||
| detection: lsst.pipe.tasks.multiBand.DetectCoaddSourcesTask | |||
| mergeDetections: lsst.pipe.tasks.mergeDetections.MergeDetectionsTask | |||
| deblend: lsst.pipe.tasks.deblendCoaddSourcesPipeline.DeblendCoaddSourcesSingleTask | |||
| deblend: lsst.pipe.tasks.deblendCoaddSourcesPipeline.DeblendCoaddSourcesMultiTask | |||
There was a problem hiding this comment.
I would like to see these pipelines be more documented. A comment here to the effect of "Use Scarlet for deblending following RFC-745" or something would be useful. DeblendCoaddSourcesMultiTask itself doesn't even have a docstring, so the relationship between this change and scarlet is unclear unless one already knows that "DeblendMulti" means "scarlet".
python/lsst/pipe/tasks/multiBand.py
Outdated
| @@ -353,7 +353,7 @@ class DeblendCoaddSourcesConfig(Config): | |||
| doc="Deblend sources separately in each band") | |||
| multiBandDeblend = ConfigurableField(target=ScarletDeblendTask, | |||
| doc="Deblend sources simultaneously across bands") | |||
| simultaneous = Field(dtype=bool, default=False, doc="Simultaneously deblend all bands?") | |||
| simultaneous = Field(dtype=bool, default=True, doc="Simultaneously deblend all bands?") | |||
There was a problem hiding this comment.
The doc for this field needs to say that this used to switch between the singleBandDeblend and multiBandDeblend subtasks listed above.
fred3m
force-pushed
the
tickets/DM-28323
branch
from
a5908e6
to
8c2d547
Compare
python/lsst/pipe/tasks/multiBand.py
Outdated
| simultaneous = Field(dtype=bool, | ||
| default=True, | ||
| doc="Simultaneously deblend all bands? " | ||
| "True uses the 'ScarletDeblendTask' while False performs deblending in " |
There was a problem hiding this comment.
I think this docstring should just refer to the ConfigurableField names, not what they are default-configured to. You also need () around this docstring, because I'm pretty sure doc= is only being assigned to the first line here.
There was a problem hiding this comment.
I don't think you need parens around string concatenation. Looks fine to me.
There was a problem hiding this comment.
milo:~ $ python xxx.py
abcdef
milo:~ $ cat xxx.py
def p(x):
print(x)
p("abc"
"def")
There was a problem hiding this comment.
Thanks, I never quite figured out when it was and wasn't necessary. I still recommend the docstring change.
fred3m
force-pushed
the
tickets/DM-28323
branch
from
8c2d547
to
c012e7b
Compare
No description provided.