New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DM-22222: Set default configs for ProcessCcd pipeline #327
Conversation
d8c6c87
to
d2fa41c
Compare
pipelines/ProcessCcd.yaml
Outdated
class: lsst.pipe.tasks.calibrate.CalibrateTask | ||
config: | ||
detection.doTempLocalBackground: False | ||
deblend.maxFootprintSize: 2000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this whole file back to how it was.
python/lsst/pipe/tasks/processCcd.py
Outdated
for task in configs.keys(): | ||
if isinstance(configs[task], dict) and 'config' in configs[task].keys(): | ||
for attr, val in configs[task]['config'].items(): | ||
self.task.attr = val |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove all these changes from this file, reverting it back to what is on master.
@@ -63,11 +64,13 @@ class ProcessCcdConfig(pexConfig.Config): | |||
) | |||
|
|||
def setDefaults(self): | |||
self.isr.doWrite = False | |||
self.charImage.doWriteExposure = False | |||
self.charImage.detection.doTempLocalBackground = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go to characterizeImage.CharacterizeImageConfig and change doWriteExposure to default to False, and add detection.doTempLocalBackground = False inside the setDefaults method.
self.charImage.doWriteExposure = False | ||
self.charImage.detection.doTempLocalBackground = False | ||
self.calibrate.detection.doTempLocalBackground = False | ||
self.calibrate.deblend.maxFootprintSize = 2000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go to calibrate.CalibrateConfig and add detection.doTempLocalBackground = False and deblend.maxFootprintSize = 2000 into the setDefaults method.
@@ -63,11 +64,13 @@ class ProcessCcdConfig(pexConfig.Config): | |||
) | |||
|
|||
def setDefaults(self): | |||
self.isr.doWrite = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not need to be moved anywhere just set this back to the way it was on master
@natelust In short, your review appears to be that I should find where the class exists and go set the configs in the corresponding class, instead of setting everything in one setDefaults method. Does it mean that I should read the yaml file in all those places? |
No, you should not be reading the yaml file at all. Reset it back to what it was wit git. Once you see the setDefault method in the classes I pointed you to you will see similar lines of code to what you were looking at. Just copy those lines into the setDefaults method of the corresponding class. |
3e6a396
to
a32d06a
Compare
Barring comments for the lines I moved, this should be good to go now. |
a32d06a
to
e6ecfaa
Compare
so that commandline task and pipeline behave the same
e6ecfaa
to
2581775
Compare
No description provided.