-
Notifications
You must be signed in to change notification settings - Fork 0
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-39243: Add a factory method for OwnedImagePlanes #27
Conversation
0337a53
to
35d535a
Compare
@classmethod | ||
def from_exposure( | ||
cls, | ||
exposure: Exposure, |
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 doesn't actually use anything from the Exposure
that isn't part of MaskedImage
. Better to call this from_masked_image
and just take MaskedImage
.
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.
True, but I was hoping to avoid the .getMaskedImage()
call and pass the Exposure
object instead (which would still run). Is your preference for this also driven by your desire to stop using Exposure
whenver possible?
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 do want to reduce usage of Exposure
when it's not adding much on top of MaskedImage
, because I'd like to someday diminish its use to the point where we could drop it.
But even if we had no intention of dropping it ever, it'd still be inappropriate to use it in places where it's adding nothing to MaskedImage
, as is the case here and in the drp_tasks
branch; this is the Interface Segregation Principle. It's the same reason AccumulatorMeanStack
only deals with MaskedImage
, not Exposure
.
(Yes, the drp_tasks
branch does call getWcs
, but it also has that same WCS throughout the exposure's lifetime already in the local skyInfo
variable, so it doesn't have to.)
37b13c6
to
c285eb7
Compare
a4ff212
to
3b50fe1
Compare
3b50fe1
to
16435db
Compare
This adds a convenient factory method to generate OwnedImagePlanes.