Details
-
Type:
Improvement
-
Status: Invalid
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw
-
Labels:None
-
Team:Data Release Production
Description
persistence using afw table is prone to causing Python circular imports in python. When folks at HSC added table persistence to afw.geom.Polygon they found that this caused a circular dependency in Python. They resolved this moving Polygon a level deeper. That worked, but has the unfortunate effect of moving Polygon to a surprising namespace and making it harder to access.
This ticket asks for changes that reduce the danger of circular dependencies and allow a more natural namespace for Polygon and future classes that are persisted using afw table.
I do not fully understand the issue, but it I saw it while moving the HSC code over, and what I observed is that "import lsst.afw.geom" imported afw table, which in turn imported afw.coord.Coord, which in turn imported afw geom. I'm not sure why adding Polygon persistence caused this, nor how we got away with the current design before Polygon adding persistence.
The problem is that some afw.table classes (e.g. SourceRecord) need access to other afw classes (like Footprint or Box), but some of those classes (like Footprint) need to inherit from afw.table.io.Persistable, or (like Box) they share a Swig module with a class (e.g. Polygon) that needs to be persistable.
I think the right way to address this is to split afw.table into multiple libraries: a low-level library that just deals with POD data (enough to support persistence), and one or more higher-level library with specific subclasses like SourceRecord, AmpInfoRecord, and ExposureRecord, which need access to e.g. geometry classes. We could also do this by moving some of these classes into other existing packages (e.g. move SourceRecord to meas_base) - but in some cases that just moves the problem (afw.image and afw.detection also have both low-level and high-level components, so moving ExposureRecord there would just create new circular dependency problems).
I hadn't planned on doing this soon, in the hopes we'd have a more global plan for package reorganization first, but we could consider moving it up.
We could also address this by making afw.table-based persistence not rely on inheritance, since that's what really makes the Swig circular dependencies fatal (and this is at least partly a Python limitation, not just a Swig one, I think - two Python modules can't both import each other). I think that would be a lot more complex than what we have now, and I haven't really thought about alternative designs, though I know they're possible (since Boost.Serialiazation doesn't rely on inheritance). The problem is that it's very much a natural polymorphism problem, so to solve it without persistence essentially involves reimplementing a virtual function mechanism somehow.