DM-42461: Implement CloughTocher2D Interpolation #878
Conversation
| ) | ||
| fillValue = pexConfig.Field[float]( | ||
| doc="Constant value to fill outside of the convex hull of the good " | ||
| "pixels. A long streak of bad pixels at an edge maybe set to this " |
There was a problem hiding this comment.
Typo: "maybe" -> "may be"? That said, should this be "will be"?
There was a problem hiding this comment.
I used "may be", because I haven't defined what long means. Perhaps I could define it in terms of buffer variable and change it to "will be".
| an exhaustive search over the image. | ||
| goodpix: `dict` [`tuple` [`int`, `int`], `float`], optional | ||
| A mapping of the coordinates of the good pixels around ``badpix`` | ||
| to their values that must be included when constructing the |
There was a problem hiding this comment.
This should mention that any values in goodpix will be overwritten with the value from the image, just so it's clear which input takes precedence.
| Returns | ||
| ------- | ||
| badpix: `set` [`tuple` [`int`, `int`]] | ||
| The coordinates of the bad pixels that were interpolate over. |
There was a problem hiding this comment.
Typo: "interpolate" to "interpolated"?
| elif (x, y) in badpix: | ||
| # If (x, y) is in badpix, but did not get flagged as bad, | ||
| # raise an exception. | ||
| raise ValueError(f"Pixel ({x}, {y}) is not bad as specified by maskPlanes {maskPlanes}") |
There was a problem hiding this comment.
This function ignores entries in goodpix (it silently overwrites them with the true image value), but raises if the image doesn't match the badpix. Shouldn't these cases be treated the same way?
While thinking through this case, I realized that goodpix is also susceptible to poisoning: an entry passed in goodpix will be used even if added to badpix. That definitely needs fixed, even if the rest of the existing logic stays as is (which I'm fine with).
There was a problem hiding this comment.
The use case here is we have a collection of images that have identical mask planes, but different image planes. So we shouldn't be treating badpix (that comes from mask) in the same way as goodpix (that comes from both the mask and image planes) the same way. But yes, agree that we should check that goodpix and badpix are mutually exclusive.
tests/test_interpImageTask.py
Outdated
|
|
||
| # Check that the long streak of bad pixels have been replaced with the | ||
| # fillValue, but not the short streak. | ||
| np.testing.assert_array_equal(self.maskedimage.image[0:1, :50].array, config.fillValue) |
There was a problem hiding this comment.
Won't this only check half the column? Same question for the other test as well.
There was a problem hiding this comment.
I thought I had a reason, but have no idea now why I did that. The test passes if I just use the full column.
arunkannawadi
force-pushed
the
tickets/DM-42461
branch
3 times, most recently
from
6201973 to
3610417
Compare
This commit prevents an unnecessary deprecation warning from emit during the build.
arunkannawadi
force-pushed
the
tickets/DM-42461
branch
from
3610417 to
744affa
Compare
No description provided.