Details
-
Type:
Improvement
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw, daf_persistence, meas_algorithms, obs_base, utils
-
Labels:
-
Story Points:10
-
Epic Link:
-
Team:Alert Production
Description
Instead of using ExposureFormatter and the like it would be simpler for the butler to run FITS persistence directly. My proposal is as follows:
- In afw add Python functions readFitsWithOptions(path, options) and writeFitsWithOptions(path, options) to all classes that the butler may persist, including Exposure, MaskedImage, Image, Mask and all catalog classes,
- In those new functions the options argument is a daf.base.PropertySet containing the "additional data" provided to the butler. The functions parse that data and use that to call readFits or writeFits method appropriately.
- Note that the path argument can be anything supported by an overload of readFits or writeFits: a file path, fits::File or fits::MemFileManager.
- In daf_persistence the Fits IO functions will be simplified to call the new functions described above, and the same pair of functions can be used for image-like objects and catalogs.
A similar treatment will be required for other objects that the butler tries to persist as FITS using boost persistence.
Attachments
Issue Links
- is triggering
-
DM-15151 Set symbol visibility to hidden in pybind11 wrappers
- Done
Activity
Jim Bosch since this work is transitional, I'm wondering how far to take it. It is clearly required for the image-like classes in order to get rid of their boost formatters, since the existing Gen2 butler code uses those formatters. But is there a reason to change I/O for catalogs? The existing Gen2 butler code does not use formatters for that. If it would help Gen3 butler in some way then fine, but otherwise I prefer to make the minimal changes required to get rid of boost formatters in afw.
I don't think there's any reason to change I/O for catalogs now.
Most of the work was done on afw on this pull request: https://github.com/lsst/afw/pull/389
A small change in utils was required, though I admit I don't know why
A few comments:
- I also removed boost persistence support from meas_algorithms (e.g. PsfFormatter)
- The reader in fitsIoWithOptions originally handled the "hdu" option as well, but it turns out "hdu" is handled somewhere else (as I should have noticed when reading the old formatters). That seems weird to me but I left it.
At this point we may be able to remove boost persistence support from daf_persistence and daf_base (e.g. PropertySet's handling of pointers to Persistable objects). But that would be a different ticket. I'm not entirely sure it's safe to do, either – pex_policy makes some use of pointer-to-persistable and I have no idea if that code is actually being used.
Looks good overall; only minor comments in afw. I'd like to understand what necessitated the changes in utils and see if we can come up with something a bit less empirical. The old functions in daf_persistence may just need different names and descriptions, but something is awry there.
Thank you for a very helpful review.
Just linked in
DM-14504. Not sure how much that ticket overlaps with this one - it's more about code removal - but if you end up doing all of it, please close it appropriately.