DM-38544: Allow getCutouts to extend off the edge of chips #698
Conversation
parejkoj
force-pushed
the
tickets/DM-38544
branch
from
08a030d
to
885b446
Compare
| * falls outside this Exposure. | ||
| */ | ||
| Exposure getCutout(lsst::geom::Box2D const& box) const; | ||
| Exposure getCutout(lsst::geom::Box2I const& box) const; |
There was a problem hiding this comment.
The Box2I overload is not documented. Did you mean to use @{ and @}? (Possibly irrelevant, given my comment below.)
There was a problem hiding this comment.
There was a problem hiding this comment.
For me the question is whether there is something extra getCutout can do with a Box2D that it couldn't do with a Box2I (e.g. set some metadata field on return). If it's just a convenience for some way of getting a Box2I from a Box2D and then calling getCutout, then I don't think we want the overload.
There was a problem hiding this comment.
Thanks, yes, I meant to wrap it in @{ @}.
I added the Box2D version because that seemed like the first thing a user would want (cutout of a generic box), while the Box2I version is what you'd do with e.g. footprint.getBBox(). I figured it would be helpful for users to not have to remember to do geom.ceil(box2d), as I think it's challenging for users to remember how to convert between our various primitives. I could add the box2d version as just a python (or pybind11) interface, if that would help?
There was a problem hiding this comment.
I could add the box2d version as just a python (or pybind11) interface, if that would help?
I don't think it would. The issue is that the correct conversion from Box2D to Box2I is context-dependent no matter which language you call it in. I assume Box2I(Box2D, EdgeHandlingEnum) is accessible from Python (possibly with a string replacing the enum)? If so, requiring that users call it themselves might be the least of the evils.
There was a problem hiding this comment.
I've removed the Box2D overload. We'll see if any users want that functionality.
Not dealing with xy0/origin here, as I don't know that we actually need it for the places we'll use this. Use the new pixel getCutout in the internals of the sky getCutout.
This is useful for getting cutouts from a footprint that may extend off an edge.
parejkoj
force-pushed
the
tickets/DM-38544
branch
from
3b0a285
to
b650b91
Compare
No description provided.