-
Notifications
You must be signed in to change notification settings - Fork 20
DM-53304: Pass attribute result struct into _compute_psf #1201
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
Conversation
339bee5 to
ae81664
Compare
ae81664 to
c4ae440
Compare
jrmullaney
left a comment
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.
LGTM!
Just one comment that you may want to address if you're feeling picky! Up to you.
tests/test_calibrateImage.py
Outdated
| self.exposure.metadata["LSST ISR FLAT APPLIED"] = True | ||
|
|
||
| # Set up a basic results struct to hold exposure attribute data | ||
| self.results = pipeBase.Struct() |
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 is probably my fault. Can you be bothered to do self.results -> self.result to be consistent with what's in calibrateImage.py?
But LGTM!
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.
Happy to. I considered naming this something else more descriptive here, like attribs or attributes, as result only really makes sense in the context of CalibrateImageTask. Any preferences for consistency over descriptiveness?
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.
Plumped for self.attributes, as result is being used in other contexts in the script already.
c4ae440 to
23c855d
Compare
Here we pass the full results attributes struct into _compute_psf. This allows us to persist the background in the event that a failure arises during processing. At present, we might persist a background-subtracted exposure but not the associated background if an error occurs mid-processing.
23c855d to
a98e4ea
Compare
No description provided.