-
Notifications
You must be signed in to change notification settings - Fork 3
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-40798: Exclude sky sources from RB classification #190
Conversation
ab15b33
to
c52b683
Compare
72e9761
to
0f85a7f
Compare
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'd like to see fairly substantial changes in this package, so I would like to re-review after you have addressed my current comments.
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 forgot to add in my initial review a request to deprecate the config doRemoveSkySources
in TransformDiaSourceCatalogTask
, and remove the code that handles sky sources around line 226 of that file. Instead, you could add a check to see if any sources are flagged as sky sources, and either raise an error if any are found.
e4c07ca
to
9e4c32c
Compare
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.
Looks good! I have a few straightforward comments, mostly to make your code style match LSST standards.
9e4c32c
to
2d460be
Compare
Create a new pipelineTask that selects sources from the diaSource catalog and writes a goodSeeingDiff_candidateDiaSrc (or similar name) catalog that can be used as an input to TransiNetTask.