DM-34912: Include pixel clipping and masks in "getGainFromFlatPair" as in "measureMeanVarCov" of "cpPtcSolve" #131
Conversation
| (e.g, an amplifier). | ||
| imageProperties : `dict` | ||
| Dictionary with masked image regions, mask planes, statistic | ||
| control objects, and clipped means. |
There was a problem hiding this comment.
This should list the expected keys and their types.
| ------- | ||
| resultsDict : `dict` | ||
| Dictionary with masked image regions, mask planes, statistic | ||
| control objects, and clipped means. |
| im2Area = afwMath.binImage(im2Area, self.config.binSize) | ||
|
|
||
| im1MaskVal = exposure1.getMask().getPlaneBitMask(self.config.maskNameList) | ||
| im1StatsCtrl = afwMath.StatisticsControl(self.config.nSigmaClipPtc, |
There was a problem hiding this comment.
Do you need two stat control objects that are essentially the same? Is the concern that the mask planes will be in a different order? These are also returned, but I'm not sure I see them used elsewhere. Are they used in code that hasn't changed?
There was a problem hiding this comment.
Is there a guarantee that exposure1.getMask().getPlaneBitMask(self.config.maskNameList) will be equal to exposure2.getMask().getPlaneBitMask(self.config.maskNameList) ? Would that object be the same to be used in ratioImMaskVal = ratioIm.getMask().getPlaneBitMask(self.config.maskNameList) of getGainFromFlatPair and in diffImMaskVal = diffIm.getMask().getPlaneBitMask(self.config.maskNameList) of measureMeanVarCov?
There was a problem hiding this comment.
As for not being used after being returned, I think you are correct, thanks!
| const = np.mean((im1Area - im2Area)**2 / (im1Area + im2Area)) | ||
| gain = 1. / const | ||
| (im1Area, im1MaskVal, im1StatsCtrl, im2Area, | ||
| im2MaskVal, im2StatsCtrl, mu1, mu2) = imageProperties.values() |
There was a problem hiding this comment.
Is there a reason to have this be a dict if it's only used as imageProperties.values()? I think the ordering is guaranteed to be consistent, but a tuple would do the same thing.
If you expect to want to access these by key in the future, then ignore this comment.
There was a problem hiding this comment.
I'll change it to a tuple. I wasn't sure if a dictionary would be more compact.
No description provided.