Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-687

Un-deprecate APIs for afw.math.Background and afw.math.BackgroundList

    XMLWordPrintable

Details

    • RFC
    • Status: Implemented
    • Resolution: Done
    • DM
    • 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:

      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

          Activity

            I support this RFC, and fully agree with krzys'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.

            swinbank John Swinbank added a comment - I support this RFC, and fully agree with krzys '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.

            In the absence of any objections, I'm adopting as proposed.

            krzys Krzysztof Findeisen added a comment - In the absence of any objections, I'm adopting as proposed.

            People

              krzys Krzysztof Findeisen
              krzys Krzysztof Findeisen
              John Swinbank, Krzysztof Findeisen, Robert Lupton, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                Jenkins

                  No builds found.