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
obs_cfht/DM-7884: unification of coadd and diff datasets #18
Conversation
deepCoadd_depth, deepCoadd_bg, deepCoadd_bgRef, deepCoadd_forced + _schema modelfits + _schema deepCoadd_modelfits + _schema, deepCoadd_multiModelfits + _schema diffsources + _schema diffpsf diff crDiffim, crDiffimSrc + _schema
Datasets which are defined in obs_base may differ from those defined in the Mapper.paf file only in the template or tables entries. Remove those parts of the dataset definiton which do not differ.
deep_makeSkyMap_config: { | ||
template: "config/deep_makeSkyMap.py" | ||
python: "lsst.pipe.tasks.makeSkyMap.MakeSkyMapTask.ConfigClass" | ||
persistable: "Config" | ||
storage: "ConfigStorage" | ||
tables: raw | ||
} | ||
# dataset defined in obs_base modified in this mapper as follows: |
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.
I don't think there's any reason for obs_cfht to define this dataset differently from the other packages; I think there's one definition that should work and others would simply be broken. I think this definition is incorrect, and I imagine that means we should just let the obs_base
definition stand here.
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.
I have this modification on the individual fixes to be done for obs_cfht. I will do those next.
@@ -396,11 +335,9 @@ datasets: { | |||
tables: raw | |||
tables: raw_visit | |||
} | |||
# dataset defined in obs_base modified in this mapper as follows: |
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 definition (forced_src_schema
) looks to me like the one we'd want in obs_base
. Could you move it there and add overrides to the obs_ packages that would need one to stay the same? (I think we'll want to modify those eventually for consistency, but better to save that for another ticket so this one makes no user-visible changes).
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.
OK, I will make the obs_base change.
No description provided.