Review for DM-4881 (Statistics: assert that images have the same size) #50
Conversation
|
We are trying to keep the HSC commit unchanged correct? I'm not sure what the "lower-left" is referring to in the commit message. The NAN is coming from the fact that all the corresponding pixels in the mask are set to be ignored, correct? |
|
Thanks Bob! I don't think keeping the HSC commit unchanged is an intrinsic good -- if we can improve it by changing it, we should do so. Obviously, we don't want to waste time changing it unless it really is an improvement though. In this case, I agree that a little rewording could make it clearer -- I'll take care of it. |
|
This is my bad as I told Bob we generally try to keep the commit history the same. Perhaps we should quickly go over our HSC port "best practices" at today's standup to make sure we're all on the same page? |
|
My naive assumption would be to cherry-pick HSC commits as is and then commit LSST local fixes as separate commits. |
|
The model we've adopted is broadly comparable to our code review process: when modest changes or cleaning up is needed, they are squashed into the original commit; when new features or significant re-writing is merited, that's a separate commit. This has been working well. |
makeStatistics() assumes that image[0,0] and mask[0,0] are the same position. If mask is bigger than image, but all the unmasked area falls outside the image given this alignment, then attempting to calculate statistics will produce a NaN. We avoid this possibility by refusing to proceed if the image and mask do not have the same dimensions.
jdswinbank
force-pushed
the
tickets/DM-4881
branch
from
024944b
to
3f988a8
Compare
If the image and mask don't have the same size, we can get a NAN
result if all pixels in the lower-left are masked. This patch adds
a check on the size of all images to catch accidental user errors.