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

Switch some C++ APIs that use pex_config information from Policy to PropertySet

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      We are trying to remove usage of pex_policy in our code so that we can retire it. There are some places that use Policy to receive configuration information (via pexConfig.makePolicy) that I would like to change to use pexConfig.makePropertySet.

      The routines in question are:

      • meas_algorithms.findCosmicRays (called from pipe_tasks and meas_algorithms)
      • ip_diffim (psf matching, kernels and regularization), seemingly only calling makePolicy internally.

      The proposal is:

      1. Change the C++ to use PropertySet
      2. Update the stack code to use the new interface.
      3. Provide a wrapper Python function that looks for the old API usage and warns, before converting the Policy to PropertySet and proceeding.
      4. Formally deprecate pexConfig.makePolicy and the Policy usage for these APIs.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            I've updated the text of the RFC to reflect my current understanding on implementation.

            Show
            tjenness Tim Jenness added a comment - I've updated the text of the RFC to reflect my current understanding on implementation.
            Hide
            tjenness Tim Jenness added a comment -

            Representative PR for meas_algorithms: https://github.com/lsst/meas_algorithms/pull/178

            Show
            tjenness Tim Jenness added a comment - Representative PR for meas_algorithms: https://github.com/lsst/meas_algorithms/pull/178
            Hide
            tjenness Tim Jenness added a comment -

            I want to change the code to use PropertySet. I don't want to also have to have the C++ code lying around for Policy (nothing in our code will be calling it)

            Show
            tjenness Tim Jenness added a comment - I want to change the code to use PropertySet. I don't want to also have to have the C++ code lying around for Policy (nothing in our code will be calling it)
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            I think for backward compatibility you would need to support both Policy and PropertySet in both C++ and Python, so the former. It would make migration when Policy is deleted simpler, too.

            Show
            krzys Krzysztof Findeisen added a comment - - edited I think for backward compatibility you would need to support both Policy and PropertySet in both C++ and Python, so the former. It would make migration when Policy is deleted simpler, too.
            Hide
            tjenness Tim Jenness added a comment -

            Warning from makePolicy won't stop the command breaking though. Are we saying I need to keep the old C++ routine around and also write a new one that takes a PropertySet? Or do I write a wrapper that notices the Policy, issues the warning, and then converts the Policy to PropertySet before using the updated C++ routine?

            Show
            tjenness Tim Jenness added a comment - Warning from makePolicy won't stop the command breaking though. Are we saying I need to keep the old C++ routine around and also write a new one that takes a PropertySet? Or do I write a wrapper that notices the Policy, issues the warning, and then converts the Policy to PropertySet before using the updated C++ routine?
            Hide
            ktl Kian-Tat Lim added a comment - - edited

            I suspect that a deprecation warning from makePolicy will be sufficient; no other means of creating a Policy to pass to these routines is likely to exist (except for test cases).

            Show
            ktl Kian-Tat Lim added a comment - - edited I suspect that a deprecation warning from makePolicy will be sufficient; no other means of creating a Policy to pass to these routines is likely to exist (except for test cases).
            Hide
            Parejkoj John Parejko added a comment -

            I'm a fan of any unneeded code removal plans. I like your python-wrapper deprecation plan.

            Show
            Parejkoj John Parejko added a comment - I'm a fan of any unneeded code removal plans. I like your python-wrapper deprecation plan.

              People

              • Assignee:
                tjenness Tim Jenness
                Reporter:
                tjenness Tim Jenness
                Watchers:
                Colin Slater, John Parejko, John Swinbank, Kian-Tat Lim, Krzysztof Findeisen, Leanne Guy, Michelle Butler, Tim Jenness, Wil O'Mullane
              • Votes:
                0 Vote for this issue
                Watchers:
                9 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Planned End:

                  Summary Panel