Fix Version/s: None
Component/s: jointcal, meas_base, pipe_drivers, pipe_tasks, skymap
Our stack has a very few bits of code that still use the geom package. In particular they use the convexHull function. geom has been superseded by sphgeom (which is written in C++ instead of python) and contains ConvexPolygon.convexHull. Update the code, paying careful attention to API differences, if any.
This is driven by a desire to repurpose the geom package for afw geometry primitives (
RFC-460) and to retire obsolete and unmaintained code.
- is blocked by
DM-14255 Add ConvexPolygon.intersects and related methods
DM-13905 Change SpherePoint.getVector to return a sphgeom UnitVector3d
- is triggered by
RFC-460 Replace the lsst.geom package with components from lsst.afw.geom
- is triggering
DM-14528 w_2018_19 & w_2018_20 coaddDriver is broken
- relates to
DM-13727 Remove skypix from stack dependencies
Could you please factor out the code duplication in the skymap tests you added?
Paul Price good suggestion. I centralized the tests in BaseSkyMapTestCase and factored common code into other methods of that class.
I also found test_equatMap.py was not using BaseSkyMapTestCase and changed that (requiring some work in the base class test case). This exposed a bug in BaseSkyMap that I fixed. Unfortunately I don't know how to unit test it because it relies on float equality. Fortunately test_equatMap.py seems to trigger it reliably.
Please have yet another look. The only changes are in skymap. Sorry for bouncing it back to you. It seems a poor way of thanking you for your good suggestions. But the changes are large enough that I think another quick look, at least, is wise.
I have not rebased the changes yet, in the interest of making it easier to see the new changes.
Paul Price Thank you for the very helpful review. I'm sorry the review turned into a marathon, but the code is far better for your suggested changes.
Paul Price I took your excellent advice to centralize the code. This consisted of two changes:
1) Made imageOverlapsTract a public function in meas_base. This should be rendered obsolete by the gen3 butler, but until then it's useful to have just one copy, as you said.
2) Added the ability to get a sky polygon from lsst.skymap.TractInfo and PatchInfo (for both inner and outer regions of each). Jim Bosch recently added something like this to skymap and I negotiated a change to add flexibility and clarity. I also added a unit test for that.
I also further refined imageOverlapsTract a bit, e.g. converting all image corners from pixels to sky in a single call to SkyWcs.pixelToSky.
Please have another look. skymap changed significantly, meas_base and jointcal both changed a bit.