Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:None
Description
In 2012, several changes were introduced (commits lsst/afw@fd4b25a, b14beb7 and 883e411) to the lsst.afw.math.Background API to try to make it less stateful and more based on explicit arguments. The following components (C++, unless otherwise stated) were noted as deprecated in the API documentation:
- BackgroundControl constructors that take Interpolate::Style or a style string
- The zero-argument form of Background.getImage (getImageF in Python)
- BackgroundMI.getPixel (all overloads)
- use of Python's BackgroundList with self-contained Background objects instead of tuples of Background and configuration information
Over the following seven years, little attempt was made to actually migrate code that used Background from the deprecated API to the new one. In DM-17566, I applied our then-new deprecation policy to these and other afw components, causing afw to emit warnings when the old APIs are used. Per policy, the APIs are currently slated for removal in Science Pipelines release 21 (DM-22814).
While migration was straightforward for most of the components affected by DM-17566, the changes for Background proved very hard to implement. Among other problems:
- Much of the old code relied on default values for style parameters in BackgroundControl. Replacing these defaults with explicit arguments to Background.getImage can be difficult, especially when getImage is called in a context far removed from the original background configuration. (
DM-17566,DM-23113) - The code's current hybrid state makes it very easy to introduce both human error and consistency bugs, since different (and sometimes multiple) classes are responsible for each aspect of Background configuration. For example, interpolation style is now handled differently from undersampling style. (
DM-23113) - The new BackgroundList API is very difficult to use correctly. (
DM-23056) - Changes to where interpolation style is handled cascade to other APIs (because they start or stop being responsible for styles). Many of these were not originally marked as deprecated. (
DM-22814,DM-23076)
I propose that we take these APIs out of deprecated status, on the grounds that the consequences of the migration were not fully appreciated and planned for when it was first proposed. This will not solve all our problems with Background, of course, but I think the previous status quo is better than the current situation.
The specific changes needed to implement this RFC are:
- revert lsst/afw@7038c57, 488744f, and 29ec3d3
- modify the pre-
DM-17566documentation to no longer describe these constructors, methods, or behaviors as deprecated - close
DM-23076andDM-23113as obsolete, and note onDM-22814that Background changes are no longer in its scope
I suggest we don't try to revert changes to bring code into compliance with the "new" APIs; since these APIs existed prior to DM-17566, they will still work.
Attachments
Issue Links
- is triggering
-
DM-24565 Un-deprecate old APIs related to afw.math.Background
- Done
- relates to
-
DM-17566 Formally deprecate discouraged C++ afw/geom components
- Done
-
DM-22814 Remove afw APIs deprecated in DM-17566
- Done
-
DM-22815 Do release 21.0.0 of science pipelines
- Done
-
DM-23056 Suppress FutureWarnings from LSST code
- Done
-
DM-23076 Mark SubtractBackgroundTask.fitBackground(..., algorithm=...) as deprecated
- Invalid
-
DM-23113 afwMath.Background.getImageF() unexpectedly ignores controller's undersample setting
- Invalid
I support this RFC, and fully agree with Krzysztof Findeisen's assessment. Trying to remove usage of the APIs that were deprecated back in 2012 has made our code more fragile and confusing than it was before.