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-18000: Implement PipelineTask and yaml file to convert DiaSources for SDM system inside ap_association #104
Conversation
Add DiaSource.yaml. Expand transform pipeline task. Add self to run method. Add to all Debug pipeline and functor yaml.
|
||
def getFunctors(self): | ||
funcs = CompositeFunctor.from_file(self.config.functorFile) | ||
funcs.update(dict(PostprocessAnalysis._defaultFuncs)) |
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.
Is this the order you want here? This would mean that PostprocessAnalysis._defaultFuncs
take precedence over what's in the functorFile
? That seems like unexpected behavior to me. Practically I see that it seems like it just adds coord_ra
and coord_dec
columns, so it probably doesn't matter; this just caught my attention.
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.
See my reply a few comments down.
diaSourceDf["diaObjectId"] = 0 | ||
diaSourceDf["htmId20"] = 0 | ||
|
||
df = self.transform(band, |
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.
self.transform()
returns the struct containing both df
and analysis
. Is the analysis
part of it used anywhere?
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.
Analysis in this context is a PostprocessAnalysis
(https://github.com/lsst/pipe_tasks/blob/0a6ae7924aec43e43d9a5ca4990e8100581c9634/python/lsst/pipe/tasks/postprocess.py#L341) object that is used by the TransformBaseCatalog
and inherited classes to run the functors. I don't see a reason for returning it as it's a class to run the functors and clean up if requested.
funcs.update(dict(PostprocessAnalysis._defaultFuncs)) | ||
return funcs | ||
|
||
# Below are temporary functions to preserve functionality before |
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'm assuming this comment means the methods below are used in some other code somewhere for the time being? If not, they should probably be removed.
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.
Correct. You just caught me before I got rid of them. Check the compare again and you'll see that this and getFunctors
is now gone and inherited from the TransformBaseCatalogTask
in postprocess.py in pipe_tasks
.
Fix bug in unittest. Fully debug unittest. Inhereit from TransformCatalogBaseTask. Fix linting. Add task description. Remove duplicated function code. Fix linting.
remove pipelineTask inherit.
0effe0b
to
eb4ead3
Compare
No description provided.