DM-17428: interpImage: allow transposition before interpolation #273
Conversation
|
|
||
| def transposeImage(image): | ||
| """Transpose an image""" | ||
| result = type(image)(transposeBox(image.getBBox())) |
There was a problem hiding this comment.
I thought the stacky way of getting an image of the same type was:image.Factory(transposeBox(image.getBBox()))
There was a problem hiding this comment.
In particular type doesn't work with proxies, although I'm not sure if they'll reappear
| def transposeImage(image): | ||
| """Transpose an image""" | ||
| result = type(image)(transposeBox(image.getBBox())) | ||
| result.array[:] = image.array.T |
There was a problem hiding this comment.
Do you really need the [:]? (See lines 229-231)
| import lsst.pex.config as pexConfig | ||
| import lsst.afw.geom as afwGeom |
There was a problem hiding this comment.
lsst.geom preferred over lsst.afw.geom
| useImage = maskedImage | ||
| useDefects = defects | ||
| if self.config.transpose: | ||
| useImage = afwImage.makeMaskedImage(transposeImage(maskedImage.image), |
There was a problem hiding this comment.
Maybe add an inline comment that this doesn't modify maskedImage
There was a problem hiding this comment.
I think that should be obvious from the code. If you disagree, let me know and I'll write something.
There was a problem hiding this comment.
👍 Without the try/finally block, it's obvious. (The finally block had me thinking that you thought you had destroyed maskedImage)
| transposeImage(maskedImage.mask), | ||
| transposeImage(maskedImage.variance)) | ||
| useDefects = ipIsr.transposeDefectList(defects) | ||
| try: |
There was a problem hiding this comment.
Unclear to me why you put this in a try/finally block here. If the code in the with block that uses this context throws an exception, yield rethrows it (regardless if the try block is here or not). If that happens, what's the point of putting the pre-interpolation pixels back in? This isn't restoring some previous state.
PaulPrice
force-pushed
the
tickets/DM-17428
branch
2 times, most recently
from
284238f
to
1b7f6ff
Compare
This allows the interpolation to act over columns instead of rows.
PaulPrice
force-pushed
the
tickets/DM-17428
branch
from
1b7f6ff
to
277928e
Compare
This allows the interpolation to act over columns instead of rows.