Tickets/dm 8108 #79
Tickets/dm 8108 #79
Conversation
| @@ -174,7 +169,7 @@ void PixelFlagsAlgorithm::measure( | |||
|
|
|||
| // Check for bits set in the source's Footprint | |||
| afw::detection::Footprint const & footprint(*measRecord.getFootprint()); | |||
| func.apply(footprint); | |||
| footprint.getSpans()->clippedTo(mimage.getBBox())->applyFunctor(func, *(mimage.getMask())); | |||
There was a problem hiding this comment.
This new syntax seems much more clumsy.
There was a problem hiding this comment.
This new syntax exists for two reasons. 1. Most of this sort of work is attempted to be part of the new SpanSet class, such that in many places people can just use SpanSets, and not carry around the extra bits of footprints. I attempted to change the minimal amount in the stack to ensure maximum compatibility, and thus I did not go all in on where SpanSets could be used exclusively. Future code would probably not be used in this manor. 2. Previously the footprint functors silently dropped locations that were in the footprint but outside images, arrays, etc. This is a dangerous thing, but was commonly exploited by code that dilated the footprint such that the bounds were outside the image. For better safety it is now required that the programmer explicitly acknowledge that they wanted these pixels dropped with a clippedTo call.
| psfImage->getArray(), | ||
| psfImage->getXY0() | ||
| ) | ||
| fitRegion.getSpans()->flatten(psfImage->getArray(), psfImage->getXY0()) |
src/PsfFlux.cc
Outdated
| (afw::image::MaskPixel const & bitPattern) | ||
| {return bitPattern & badBits;}); | ||
| fitRegion.setSpans(fitRegion.getSpans()->intersectNot(*tmpSpans)->clippedTo( | ||
| exposure.getMaskedImage().getMask()->getBBox())); |
There was a problem hiding this comment.
Another case where the syntax seems much more clumsy than before.
There was a problem hiding this comment.
I'm glad you pointed this out, as it was partly left over from an intermediate development stage. The code now reads:
fitRegion.setSpans(fitRegion.getSpans()->intersectNot(*exposure.getMaskedImage().getMask(), badBits)->clippedTo(exposure.getMaskedImage().getMask()->getBBox()))
If you ignore the fact that this is sitting inside a footprint, this is exactly the same syntax but now on a spanset object. This SpanSet just must be accessed, and set to, the footprint object, as it itself is immutable. We may consider an api update in the future to add a convenience method to do with from the footprint if it proves necessary. We are hoping that SpanSets will take over in newly developed code such that this sort of thing will be quite rare.
There was a problem hiding this comment.
If you think an API update is worth it, please file a ticket now so we don't forget. If you suspect it will likely be unneeded, don't bother with the ticket.
| source.setFootprint(afwDetection.Footprint(afwGeom.Point2I(afwGeom.Point2D(x + x0, y + y0)), 5)) | ||
| tmpSpanSet2 = afwGeom.SpanSet.fromShape(5).shiftedBy(x + x0, | ||
| y + y0) | ||
| source.setFootprint(afwDetection.Footprint(tmpSpanSet2)) |
There was a problem hiding this comment.
Ah the joys of 79 characters...
| spans.append(ss) | ||
| for ss in foot2.getSpans(): | ||
| spans.append(ss) | ||
| spans = foot1.spans.union(foot2.spans) |
natelust
force-pushed
the
tickets/DM-8108
branch
from
c4feed0
to
cfd38fe
Compare
No description provided.